Re: Review Request: Multiple actions support for Nepomuk Search Runner
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/689/#review1764 --- Ship it! looks ok, and yes the camel case includes should be added. trunk/KDE/kdebase/workspace/plasma/runners/nepomuksearch/nepomuksearchrunner.cpp http://reviewboard.kde.org/r/689/#comment1151 is this actually needed? shouldn't the catalog be automatically loaded? trunk/KDE/kdebase/workspace/plasma/runners/nepomuksearch/queryclientwrapper.cpp http://reviewboard.kde.org/r/689/#comment1152 no spaces in the ( )s - Aaron On 2009-07-25 04:50:49, Ryan Bitanga wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/689/ --- (Updated 2009-07-25 04:50:49) Review request for Plasma. Summary --- Reposting patch for multiple actions support for the Nepomuk search runner. It adds open with and service menu actions to the nepomuk search runner. Now uses the new KFileItemActions and KFileItemListProperties and no longer depends on libkonq. Minor style issue: the two new classes in KIO don't have camel case includes yet, and I wasn't sure if I should add them myself. Another possible point for improvement: move the parseMenu method to AbstractRunner as something like an addActionsFromMenu method to reuse code between runners (useful for the locations runner and the window management runner which I'm thinking of moving into kdereview) Diffs - trunk/KDE/kdebase/workspace/plasma/runners/nepomuksearch/nepomuksearchrunner.h 999602 trunk/KDE/kdebase/workspace/plasma/runners/nepomuksearch/nepomuksearchrunner.cpp 999602 trunk/KDE/kdebase/workspace/plasma/runners/nepomuksearch/queryclientwrapper.cpp 999602 Diff: http://reviewboard.kde.org/r/689/diff Testing --- Thanks, Ryan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Allow AbstractRunner to parse QMenus for actions
On 2009-07-25 05:24:44, Aaron Seigo wrote: does this really belong in AbstractRunner? obviously, it's runners that are currently using it, but this method doesn't actually have anything to do with the functionality AbstractRunner provides. maybe it should be added to plasma.h instead? I couldn't think of any other use case so I opted to keep it as a protected function of AbstractRunner. I'm not sure having a public function for retrieving a list of actions would be useful to any other application but if anyone can think of another use for it, we could move it to plasma.h - Ryan --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/#review1760 --- On 2009-07-25 01:24:26, Ryan Bitanga wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/ --- (Updated 2009-07-25 01:24:26) Review request for Plasma. Summary --- This adds a convenience method to AbstractRunner to extract a list of actions from a QMenu. Hierarchy is preserved by prefixing submenu text to the actions in the submenus. Current uses are extracting actions from KFileItemActions and libtaskmanager. The code is currently used by the window management runner in playground, the multiple action nepomuk search runner patch in reviewboard, and fsrunner in kde-apps.org. This method will help avoid code duplication in these runners as well as future runners that can extract info from menus. Diffs - trunk/KDE/kdelibs/plasma/abstractrunner.h 1001909 trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1001909 Diff: http://reviewboard.kde.org/r/1115/diff Testing --- Thanks, Ryan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Allow AbstractRunner to parse QMenus for actions
On 2009-07-25 05:24:44, Aaron Seigo wrote: does this really belong in AbstractRunner? obviously, it's runners that are currently using it, but this method doesn't actually have anything to do with the functionality AbstractRunner provides. maybe it should be added to plasma.h instead? Ryan Bitanga wrote: I couldn't think of any other use case so I opted to keep it as a protected function of AbstractRunner. I'm not sure having a public function for retrieving a list of actions would be useful to any other application but if anyone can think of another use for it, we could move it to plasma.h it's not about having other uses of the method, it's simply that this method has _nothing_ to do with AbstractRunner. it's just a coincidence that only runners are using this right now. since we can change API after it's introduced, this concerns me. :) if it does stay in AbstractRunner and we later move it, then the method in AbstractRunner would become a wrapper around the one in the Plasma namespace. i don't see any reason not to just it in Plasma:: in the first place, and then we don't need to worry if others will want to use it in the future or not: it's already somewhere everyone can get at it. - Aaron --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/#review1760 --- On 2009-07-25 01:24:26, Ryan Bitanga wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/ --- (Updated 2009-07-25 01:24:26) Review request for Plasma. Summary --- This adds a convenience method to AbstractRunner to extract a list of actions from a QMenu. Hierarchy is preserved by prefixing submenu text to the actions in the submenus. Current uses are extracting actions from KFileItemActions and libtaskmanager. The code is currently used by the window management runner in playground, the multiple action nepomuk search runner patch in reviewboard, and fsrunner in kde-apps.org. This method will help avoid code duplication in these runners as well as future runners that can extract info from menus. Diffs - trunk/KDE/kdelibs/plasma/abstractrunner.h 1001909 trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1001909 Diff: http://reviewboard.kde.org/r/1115/diff Testing --- Thanks, Ryan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Allow AbstractRunner to parse QMenus for actions
On 2009-07-25 05:24:44, Aaron Seigo wrote: does this really belong in AbstractRunner? obviously, it's runners that are currently using it, but this method doesn't actually have anything to do with the functionality AbstractRunner provides. maybe it should be added to plasma.h instead? Ryan Bitanga wrote: I couldn't think of any other use case so I opted to keep it as a protected function of AbstractRunner. I'm not sure having a public function for retrieving a list of actions would be useful to any other application but if anyone can think of another use for it, we could move it to plasma.h Aaron Seigo wrote: it's not about having other uses of the method, it's simply that this method has _nothing_ to do with AbstractRunner. it's just a coincidence that only runners are using this right now. since we can change API after it's introduced, this concerns me. :) if it does stay in AbstractRunner and we later move it, then the method in AbstractRunner would become a wrapper around the one in the Plasma namespace. i don't see any reason not to just it in Plasma:: in the first place, and then we don't need to worry if others will want to use it in the future or not: it's already somewhere everyone can get at it. Alright, I see where you're coming from. :) Moved it to plasma.h - Ryan --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/#review1760 --- On 2009-07-25 01:24:26, Ryan Bitanga wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1115/ --- (Updated 2009-07-25 01:24:26) Review request for Plasma. Summary --- This adds a convenience method to AbstractRunner to extract a list of actions from a QMenu. Hierarchy is preserved by prefixing submenu text to the actions in the submenus. Current uses are extracting actions from KFileItemActions and libtaskmanager. The code is currently used by the window management runner in playground, the multiple action nepomuk search runner patch in reviewboard, and fsrunner in kde-apps.org. This method will help avoid code duplication in these runners as well as future runners that can extract info from menus. Diffs - trunk/KDE/kdelibs/plasma/abstractrunner.h 1001909 trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1001909 Diff: http://reviewboard.kde.org/r/1115/diff Testing --- Thanks, Ryan ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix layout problems in the pager applet
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/#review1761 --- /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp http://reviewboard.kde.org/r/1124/#comment1149 these lines must not be removed; recalculateGeometry() may cause a change in the size of the applet, which in turn causes constraintsEvent to get called with SizeConstraint again which then call recalculateGeometry .. great way to trigger an infinite loop ;) /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp http://reviewboard.kde.org/r/1124/#comment1150 how well does this work with changing a panel from horizontal to vertical and back again? the user should not need to reconfigure the pager for it to look nice. - Aaron On 2009-07-25 01:11:34, Anthony Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/ --- (Updated 2009-07-25 01:11:34) Review request for Plasma. Summary --- This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013. Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space. This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows 1 My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place. This addresses bug 200013. https://bugs.kde.org/show_bug.cgi?id=200013 Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 Diff: http://reviewboard.kde.org/r/1124/diff Testing --- Tested in vertical and horizontal panels and on the desktop. Thanks, Anthony ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
containing plasmoid crashes
Hello guys, I'm writing to you because I'd like to make a suggestion to make a containment for errors around each plasmoid, so that when one crashes, it doesn't take the whole plasma environment with it. For example, I had a problem with KDE Network Manager plasmoid crashing and taking with it the whole plasma-desktop process before displaying anything. I wouldn't even get to see the panel containing the offending plasmoid, so the only workaround I could find was to 'bastardize' the plasma config files to remove the plasmoid from the panel. After removing it from the panel plasma works fine. I am not writing this mail to whine about my issue: I think building a fence/containment for errors around each plasmoid should solve a lot of bugs in Bugs.Kde.Org, or even if it doesn't fix the plasmoid errors, this should make them bearable for end-users: they can ignore or remove the offending plasmoid and be merry with it. There's also the issue of reporting krashes in BKO: if a containment is build around plasmoids, crashes will not be caught by Dr Krash. But can't you launch Krash from the plasma containment to build the report? -- The best way to predict the future is to invent it., 1971, Alan Kay: http://www.smalltalk.org/alankay.html ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Gethotnewstuff and themes
On Thursday 23 July 2009, Aaron J. Seigo wrote: * use plasmapkg and let it do the dirty work; this is probably the proper method. done. :) -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Software signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: containing plasmoid crashes
On Saturday 25 July 2009, Bogdan Bivolaru wrote: I'm writing to you because I'd like to make a suggestion to make a containment for errors around each plasmoid, so that when one crashes, it doesn't take the whole plasma environment with it. and how do you suggest this is accomplished, exactly? -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Software signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix layout problems in the pager applet
On 2009-07-25 08:56:47, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp, lines 261-264 http://reviewboard.kde.org/r/1124/diff/1/?file=9019#file9019line261 how well does this work with changing a panel from horizontal to vertical and back again? the user should not need to reconfigure the pager for it to look nice. I see what you mean. A fix shouldn't be too hard, but this isn't really the place to do it. If I could swap rows and columns whenever the formfactor changes to/from vertical it would work properly. Is there a way to get the last form factor we were in from a constraints event or should I store it in the applet? On 2009-07-25 08:56:47, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp, lines 147-150 http://reviewboard.kde.org/r/1124/diff/1/?file=9019#file9019line147 these lines must not be removed; recalculateGeometry() may cause a change in the size of the applet, which in turn causes constraintsEvent to get called with SizeConstraint again which then call recalculateGeometry .. great way to trigger an infinite loop ;) This was my main concern. The reasons I thought it might be ok to do this were: 1. it doesn't actually resize(), only setPreferredSize() and setMinimumSize() 2. as long as the height (or in vertical panels: width) doesn't change, it will only setPreferredSize() and setMinimumSize() to the same thing it set them to last time. The problem with doing it like this is that half of the size constraints events will be ignored, which means that if the right number of events are generated there could be layout problems afterwards. e.g. if the size shrinks there will be clipping errors. - Anthony --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/#review1761 --- On 2009-07-25 01:11:34, Anthony Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/ --- (Updated 2009-07-25 01:11:34) Review request for Plasma. Summary --- This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013. Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space. This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows 1 My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place. This addresses bug 200013. https://bugs.kde.org/show_bug.cgi?id=200013 Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 Diff: http://reviewboard.kde.org/r/1124/diff Testing --- Tested in vertical and horizontal panels and on the desktop. Thanks, Anthony ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Moving libconversation to kdereview
On Thursday 23 July 2009, Carsten Niehaus wrote: Moin moin With the commits r1001601, r1001602,r1001603 and r1001608 I moved the lib to kde-review. I hope I did everything correctly... I never moved a lib... There is one CMake-issue left which I am unable to fix: For some reasons I am getting this issue: kdeplasma-addons/libs/plasmaweather/weatherpopupapplet.cpp:29:34: error: conversion/converter.h: No such file or directory snip kdeplasma-addons/libs/plasmaweather/weatherpopupapplet.cpp:34: error: 'Conversion' is not a namespace-name The files are installed in kde/include/conversion/. I don't know how I can teach plasmaweather to find the files... Can somebody on this list please help? Let the review begin! Carsten as an emergency measure i made stuff depending from that build conditionally -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix layout problems in the pager applet
On 2009-07-25 08:56:47, Aaron Seigo wrote: /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp, lines 147-150 http://reviewboard.kde.org/r/1124/diff/1/?file=9019#file9019line147 these lines must not be removed; recalculateGeometry() may cause a change in the size of the applet, which in turn causes constraintsEvent to get called with SizeConstraint again which then call recalculateGeometry .. great way to trigger an infinite loop ;) wrote: This was my main concern. The reasons I thought it might be ok to do this were: 1. it doesn't actually resize(), only setPreferredSize() and setMinimumSize() 2. as long as the height (or in vertical panels: width) doesn't change, it will only setPreferredSize() and setMinimumSize() to the same thing it set them to last time. The problem with doing it like this is that half of the size constraints events will be ignored, which means that if the right number of events are generated there could be layout problems afterwards. e.g. if the size shrinks there will be clipping errors. call setPreferredSize and setMinimumSize can result in a change of size, so point 1 isn't correct. point 2 is dangerous to rely on. it's the same assumption that led to m_ignoreNextSizeConstraint being added in the first place. perhaps in recalculateGeometry, don't call any potentially resize-causing methods if m_ignoreNextSizeConstraint; just figure out values for m_rects within the bounds given. - Aaron --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/#review1761 --- On 2009-07-25 14:03:16, Anthony Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/ --- (Updated 2009-07-25 14:03:16) Review request for Plasma. Summary --- This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013. Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space. This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows 1 My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place. This addresses bug 200013. https://bugs.kde.org/show_bug.cgi?id=200013 Diffs - /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 Diff: http://reviewboard.kde.org/r/1124/diff Testing --- Tested in vertical and horizontal panels and on the desktop. Thanks, Anthony ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Fix layout problems in the pager applet
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1124/ --- (Updated 2009-07-25 21:40:41.819952) Review request for Plasma. Changes --- Tried implementing Aaron's suggestion to not call anything that could cause a resize if m_ignoreNextSizeConstraint. Unfortunately, when changing a panel between vertical and horizontal it resulted in the applet taking up excessive widths/heights because of the swap between rows and columns that happens during the form factor constraints event. Another solution that seems to work quite well is to always set the new preferred size if the height (or in vertical panels: width) has changed, so that has been implemented in this version of the patch. Summary --- This patch fixes a few problems with the pager that are especially apparent in vertical panels. Specifically, the rows and columns are unnecessarily swapped in recalculateGeometry() which causes bug 200013. Also, due to the way the rectangle sizes were calculated it was possible for certain areas to be clipped. This patch makes sure every item can fit into the current space. This also fixes a bug I only noticed when reading the code, where extra unnecessary columns could be added if desktopCount % rows 1 My main concern about this patch is that it removes a mechanism for ignoring some constraints events, I think these events are needed in order to avoid clipping some of the virtual desktops, but I'm not sure how to reproduce the bug that made this filtering necessary in the first place. This addresses bug 200013. https://bugs.kde.org/show_bug.cgi?id=200013 Diffs (updated) - /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.h 1002081 /trunk/KDE/kdebase/workspace/plasma/applets/pager/pager.cpp 1002081 Diff: http://reviewboard.kde.org/r/1124/diff Testing --- Tested in vertical and horizontal panels and on the desktop. Thanks, Anthony ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Prospective costs of the 3rd Plasma meeting (Tokamak3) - part 2
On Thursday 23 July 2009, 15:42 Ivan Čukić wrote: Ana Cecília Martins Barbosa: anaceciliamb at gmail.com Just came to my mind that she'll also need an invitation letter. I don't need as I have double citizenship and I come as an italian. But it's better for brazilians to have invitation letters when coming to europe. Cheers, -- Artur Duque de Souza openBossa Research Labs INdT - Instituto Nokia de Tecnologia -- Blog: http://blog.morpheuz.cc PGP: 0xDBEEAAC3 @ wwwkeys.pgp.net --- signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
