Re: Review Request: TaskManager: Store launcher order
On Nov. 1, 2011, 2:56 p.m., Aaron J. Seigo wrote: libs/taskmanager/strategies/desktopsortingstrategy.cpp, line 40 http://git.reviewboard.kde.org/r/103006/diff/1/?file=39936#file39936line40 this should be a private member of the class ... as this is a non-exported class, binary compatibility doesn't matter. separateLaunchers is used in the static lessThan() function. So it makes no sense to place it in the class - will not be used elsewhere. On Nov. 1, 2011, 2:56 p.m., Aaron J. Seigo wrote: libs/taskmanager/strategies/programgroupingstrategy.cpp, lines 69-73 http://git.reviewboard.kde.org/r/103006/diff/1/?file=39939#file39939line69 this check looks inccorect: if launchers are integrated or separate, it should still be possible to say that a given application should not be groupable with other windows from that same application. The another flag will have to be added to GroupManager - as for an icon only taskbar, it makes no sense to split the group. - Craig --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103006/#review7798 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103006/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. Store the order in which launchers are created 2. Add a new config option, separateLaunchers - so that applet can decide if launchers and tasks should be kept separate. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/abstractgroupingstrategy.cpp 4ed424a libs/taskmanager/abstractsortingstrategy.cpp 390f6f0 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/launcheritem.cpp 20f0e7c libs/taskmanager/strategies/alphasortingstrategy.cpp 9ec1aca libs/taskmanager/strategies/desktopsortingstrategy.cpp 520fead libs/taskmanager/strategies/manualsortingstrategy.h 113faab libs/taskmanager/strategies/manualsortingstrategy.cpp 4409a6b libs/taskmanager/strategies/programgroupingstrategy.cpp 5c43d03 libs/taskmanager/taskgroup.h 53c2871 libs/taskmanager/taskgroup.cpp 49140ae libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103006/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: TaskAction changes from IconTasks
On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskactions.cpp, line 384 http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line384 we have a name, it should be used. it hardly matters what the eventual Launcher is called. shortening it with the removal of It Is is probably alright (computer application english :) Craig Drummond wrote: Show A Launcher for Systemsettings When It is Not Running is a *very* long menu item. What advantage does adding the name here give? The user already right-clicked on the taskbar entry - so they know what the item is. Placing its name here just adds extra noise. In fact I'd actually prefer just Pin To Taskbar and Unpin From Taskbar Aaron J. Seigo wrote: a) it tells the user what is (likely) to happen with the launcher; as it doesn't actually relate directly to the window itself, this is useful information .. particularly as many times you can't even see the name of the application in the task widget button, so have nothing to really reference visually. b) pin has got to be one of the worst bits of new jargon that has come with docks. in my testing with run-of-the-mill users not used to docks they tend to fail with complete reliability at explaining what it will do it could be shortened to just Show a launcher for name though? I still disagree with adding the name. We dont have Close name, Minimise name. So why is Show A Launcher any different? On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskactions.cpp, lines 508-510 http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line508 there's a line between simplifiying and making it useless in too many cases that this crosses imho. this would have me reaching for the window titlebar in too many cases. i would be in favour of renaming Advanced (a word we are trying to get away from) with something like More actions Craig Drummond wrote: hehe! knew this would get some reactions. Its only an option anyway. Perhaps it would be better if the user could supply a mask of items they did *not* want in the basic menu? Then it's up to the taskbar applet to decide what to show? Aaron J. Seigo wrote: sounds like a great recipe for inconsistency, poor choices and the creation of new user-facing options. i'd really like to avoid it. coming to a good compromise situation that works well enough for everyone should be possible. In the next diff I've removed the 'simple' option. It needs more thought. Plus, it might be better to just re-think the context menu so that there is no need for a 'simple' mode. - Craig --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103004/#review7794 --- On Oct. 31, 2011, 8:40 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103004/ --- (Updated Oct. 31, 2011, 8:40 p.m.) Review request for Plasma. Description --- 1. Add NewInstance action to launch a new instance of an application. 2. Move toggle launcher action out of advanced menu. 3. Don't show application name in 'Show A Launcher' action, as might not know the real name at this time (e.g. no desktop file read). 4. Add application actions, to be shown at the top of the right-click menu. IconTasks uses this to show recent docuents, dock manager menu items, and unity items. 5. A ToolTipMenu class is created, so that tooltips may be give for menu items. This is so that IconTasks can display the full path of a documents in the recent documents menu. 6. Add option to have only basic window controls in menu. Diffs - libs/taskmanager/taskactions.h 2b5a641 libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskactions_p.h 913966c Diff: http://git.reviewboard.kde.org/r/103004/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskactions.cpp, line 375 http://git.reviewboard.kde.org/r/103007/diff/1/?file=39955#file39955line375 dialogs must not be exec()'d as they *freeze* the entire UI.. in this case the whole plasma-desktop shell. i must be handled asynchronously. Craig Drummond wrote: Good point! This is odd. I've tried running the dialog via show() and connecting to the okClicked() signal. However, the slot never gets called. If I change it to exec() the slot does get called... e.g. this does *not* work: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-show(); ...but this does: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-exec(); (Ignore the obvious memory leak) - However, this seems to be a general issue with plasma dialogs. e.g. if you configure a colour for the clock dialog, plasma is frozen. Perhaps all plasma dialogs need to be run via another process? - Craig --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/#review7801 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/kapplicationselectordialog.h PRE-CREATION libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION libs/taskmanager/launcherconfig.h PRE-CREATION libs/taskmanager/launcherconfig.cpp PRE-CREATION libs/taskmanager/launcherconfig.ui PRE-CREATION libs/taskmanager/launcherproperties.h PRE-CREATION libs/taskmanager/launcherproperties.cpp PRE-CREATION libs/taskmanager/launcherproperties.ui PRE-CREATION libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103007/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: KRunner Guidelines
On Tuesday, November 01, 2011 21:00:54 David Baron wrote: On Tuesday 04 Heshvan 5772 21:48:51 Aaron J. Seigo wrote: On Tuesday, November 1, 2011 19:06:12 David Baron wrote: Should a runner flag it hits. Right now my recoll runner should them as recoll-filename. Other runners show Mail to soandso or Run this-app. However, many others just yield the file name with no hint of why the user is seeing this one. Icons suggest the file's type or mime-type rather than which runner flagged it. the runner is an implementation detail. the returned search results should reflect the content of what is in the hit, rather than the plugin that generated it. OK. What do we mean by an implementation detail? What might be and what might not be? Good: - open this file - send this file to... - start app X' - Do Y with Z Bad: - Photoshop runner has found a match - recoll has found a match I.e., it should not matter which runner returned the match. -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: TaskAction changes from IconTasks
On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
libs/taskmanager/taskactions.cpp, line 384
http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line384
we have a name, it should be used. it hardly matters what the eventual
Launcher is called. shortening it with the removal of It Is is probably
alright (computer application english :)
Craig Drummond wrote:
Show A Launcher for Systemsettings When It is Not Running is a *very*
long menu item. What advantage does adding the name here give? The user
already right-clicked on the taskbar entry - so they know what the item is.
Placing its name here just adds extra noise. In fact I'd actually prefer just
Pin To Taskbar and Unpin From Taskbar
Aaron J. Seigo wrote:
a) it tells the user what is (likely) to happen with the launcher; as it
doesn't actually relate directly to the window itself, this is useful
information .. particularly as many times you can't even see the name of the
application in the task widget button, so have nothing to really reference
visually.
b) pin has got to be one of the worst bits of new jargon that has come
with docks. in my testing with run-of-the-mill users not used to docks they
tend to fail with complete reliability at explaining what it will do
it could be shortened to just Show a launcher for name though?
Craig Drummond wrote:
I still disagree with adding the name. We dont have Close name,
Minimise name. So why is Show A Launcher any different?
because close, minimize, etc. all operate on the window, while the launcher
operates on something that isn't necessarily the window.
but since we're going back and forth on this point, i'll compromise on this
point: let's try it your way without a name :)
On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
libs/taskmanager/taskactions.cpp, lines 508-510
http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line508
there's a line between simplifiying and making it useless in too
many cases that this crosses imho. this would have me reaching for the
window titlebar in too many cases.
i would be in favour of renaming Advanced (a word we are trying to
get away from) with something like More actions
Craig Drummond wrote:
hehe! knew this would get some reactions. Its only an option anyway.
Perhaps it would be better if the user could supply a mask of items they did
*not* want in the basic menu? Then it's up to the taskbar applet to decide
what to show?
Aaron J. Seigo wrote:
sounds like a great recipe for inconsistency, poor choices and the
creation of new user-facing options. i'd really like to avoid it. coming to a
good compromise situation that works well enough for everyone should be
possible.
Craig Drummond wrote:
In the next diff I've removed the 'simple' option. It needs more thought.
Plus, it might be better to just re-think the context menu so that there is
no need for a 'simple' mode.
i agree completely about re-thinking the context menu. let's get this patch in,
and then after that we can work on making the menu Not Suck(tm)
On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
libs/taskmanager/taskactions.cpp, line 600
http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line600
this is a misuse of QAction. you can add QActions to a QAction and that
parent QAction becomes the submenu. no need for such custom property
setting/reading and menu creation.
Craig Drummond wrote:
This was for dock-manager plugin support - just to make things easier on
my side - then there's a 1:1 mapping between the dockmanager item and the
QAction. No biggie, I can always remove this.
Aaron J. Seigo wrote:
hm... can't you add all the dockmanager items to a common QAction? should
be no harder than calling setProperty on them? or are these properties
already set by the dockmanager API? (in which case i'd suggest the dock
manager API could use improving :)
Craig Drummond wrote:
Hmm... How do I add a QAction to a QAction? I cant see this anywhere in
the documentation. I've tried QActionGroup, creating QActions with the
QAction as a parent. All failed :-(
QMenu *menu = new QMenu(parent);
QAction *action = menu-menuAction();
you can then add actions to the menu, and return the menuAction() as the action
for the menu. it's not the most intuitive thing (almost backwards imho :) but
it works nicely. we use it in several places in plasma to achieve exactly this
effect :)
On Nov. 1, 2011, 2:05 p.m., Aaron J. Seigo wrote:
libs/taskmanager/taskactions.cpp, line 135
http://git.reviewboard.kde.org/r/103004/diff/1/?file=39919#file39919line135
why a property for this? why not simply:
if (!act-toolTip().isEmpty()) {
Craig Drummond wrote:
Using if (!act-toolTip().isEmpty()) results in the menuitem's text
being shown as
Re: Review Request: TaskManager: Improve matching of task to launcher (.desktop file)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103005/#review7869 --- Ship it! huzzah! first patch read to go and the others not far behind .. we're making progress! :)) - Aaron J. Seigo On Nov. 1, 2011, 5:27 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103005/ --- (Updated Nov. 1, 2011, 5:27 p.m.) Review request for Plasma. Description --- Add more KTrader queries when attempting to match a task to a launcher. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/startup.h 997eb7e libs/taskmanager/startup.cpp e4df7c7 libs/taskmanager/task.h 45acd5a libs/taskmanager/task_win.cpp ea03004 libs/taskmanager/task_x11.cpp 04af0c7 libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 libs/taskmanager/taskmanagerrulesrc PRE-CREATION Diff: http://git.reviewboard.kde.org/r/103005/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Store launcher order
On Nov. 1, 2011, 2:56 p.m., Aaron J. Seigo wrote: libs/taskmanager/strategies/desktopsortingstrategy.cpp, line 40 http://git.reviewboard.kde.org/r/103006/diff/1/?file=39936#file39936line40 this should be a private member of the class ... as this is a non-exported class, binary compatibility doesn't matter. Craig Drummond wrote: separateLaunchers is used in the static lessThan() function. So it makes no sense to place it in the class - will not be used elsewhere. there are ways around this :) .. tell you what, once this is merged in i'll make some small adjustments that address this issue to improve code clarity. On Nov. 1, 2011, 2:56 p.m., Aaron J. Seigo wrote: libs/taskmanager/strategies/programgroupingstrategy.cpp, lines 69-73 http://git.reviewboard.kde.org/r/103006/diff/1/?file=39939#file39939line69 this check looks inccorect: if launchers are integrated or separate, it should still be possible to say that a given application should not be groupable with other windows from that same application. Craig Drummond wrote: The another flag will have to be added to GroupManager - as for an icon only taskbar, it makes no sense to split the group. another flag is fine, and in this case it will keep feature sets properly seperated. with that flag in, i think this patch will be ready to go. - Aaron J. --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103006/#review7798 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103006/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. Store the order in which launchers are created 2. Add a new config option, separateLaunchers - so that applet can decide if launchers and tasks should be kept separate. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/abstractgroupingstrategy.cpp 4ed424a libs/taskmanager/abstractsortingstrategy.cpp 390f6f0 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/launcheritem.cpp 20f0e7c libs/taskmanager/strategies/alphasortingstrategy.cpp 9ec1aca libs/taskmanager/strategies/desktopsortingstrategy.cpp 520fead libs/taskmanager/strategies/manualsortingstrategy.h 113faab libs/taskmanager/strategies/manualsortingstrategy.cpp 4409a6b libs/taskmanager/strategies/programgroupingstrategy.cpp 5c43d03 libs/taskmanager/taskgroup.h 53c2871 libs/taskmanager/taskgroup.cpp 49140ae libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103006/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: the wholesale copying of KOpenWithDialog is very unfortunate and needs to be avoided if at *all* possible. why is it copied instead of used directly? the really big issue is the exec()'ing of the dialog, however. it would also be nice to be able to access individual launcher configuration via the individual launchers' context menu. p.s. whitespace :) Craig Drummond wrote: I agree the copying is bad - but without changing kdelibs I knew of no other way. The reason it is copied is that in KOpenWithDialog you cannot remove the Open with label (or change its text), or the kurlrequester below it. the Open With label is settable by using the KOpenWithDialog( const KUrl::List urls, const QString text, const QString value, QWidget *parent = 0 ); constructor, so it could be set to something sensible for this case. as forthe KUrlRequester ... is that so bad? it allows the user to define an arbitrary exec on disk, the ultimate last resort. On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskitem.cpp, line 362 http://git.reviewboard.kde.org/r/103007/diff/1/?file=39957#file39957line362 in which cases would the mapping not be saved (e.g. saveMapping == false)? Craig Drummond wrote: In another patch (not done yet), LauncherItem::associateItemIfMatches() calls setLauncherUrl on the task. But as this was not done by the user, I didnt want to save the mapping. ah! well, this could be accomplished with a LauncherItem::setLauncherUrl(LauncherItem *item) overload. code duplication would be minimal (just the first 4 lines of code, really) and the API would be clearer imo. On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskactions.cpp, line 375 http://git.reviewboard.kde.org/r/103007/diff/1/?file=39955#file39955line375 dialogs must not be exec()'d as they *freeze* the entire UI.. in this case the whole plasma-desktop shell. i must be handled asynchronously. Craig Drummond wrote: Good point! Craig Drummond wrote: This is odd. I've tried running the dialog via show() and connecting to the okClicked() signal. However, the slot never gets called. If I change it to exec() the slot does get called... e.g. this does *not* work: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-show(); ...but this does: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-exec(); (Ignore the obvious memory leak) - However, this seems to be a general issue with plasma dialogs. e.g. if you configure a colour for the clock dialog, plasma is frozen. Perhaps all plasma dialogs need to be run via another process? e.g. this does *not* work that is very, very odd. it would seem to imply that there is an event loop issue. there is no difference otherwise between exec() and show() (in fact, in fixing kcolorbutton i use show()). ah! perhaps i see the issue: LauncherProperties::run calls exec() on itself .. that creates an inner event loop around LauncherProperties. my guess is that trips it up. neither LauncherProperties nor KApplicationSelectorDialog should be exec()'d (Ignore the obvious memory leak) btw, easiest way to deal with this is to call dlg-setAttribute(Qt::WA_DeleteOnClose); this seems to be a general issue with plasma dialog we have this issue in very few places, and where it remains it is fixable .. i just fixed KColorButton right now in fact (thanks for pointing that one out) - Aaron J. --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/#review7801 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/kapplicationselectordialog.h PRE-CREATION libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskitem.cpp, line 362 http://git.reviewboard.kde.org/r/103007/diff/1/?file=39957#file39957line362 in which cases would the mapping not be saved (e.g. saveMapping == false)? Craig Drummond wrote: In another patch (not done yet), LauncherItem::associateItemIfMatches() calls setLauncherUrl on the task. But as this was not done by the user, I didnt want to save the mapping. Aaron J. Seigo wrote: ah! well, this could be accomplished with a LauncherItem::setLauncherUrl(LauncherItem *item) overload. code duplication would be minimal (just the first 4 lines of code, really) and the API would be clearer imo. OK, no biggie. I'll refactor this. - Craig --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/#review7801 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/kapplicationselectordialog.h PRE-CREATION libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION libs/taskmanager/launcherconfig.h PRE-CREATION libs/taskmanager/launcherconfig.cpp PRE-CREATION libs/taskmanager/launcherconfig.ui PRE-CREATION libs/taskmanager/launcherproperties.h PRE-CREATION libs/taskmanager/launcherproperties.cpp PRE-CREATION libs/taskmanager/launcherproperties.ui PRE-CREATION libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103007/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: the wholesale copying of KOpenWithDialog is very unfortunate and needs to be avoided if at *all* possible. why is it copied instead of used directly? the really big issue is the exec()'ing of the dialog, however. it would also be nice to be able to access individual launcher configuration via the individual launchers' context menu. p.s. whitespace :) Craig Drummond wrote: I agree the copying is bad - but without changing kdelibs I knew of no other way. The reason it is copied is that in KOpenWithDialog you cannot remove the Open with label (or change its text), or the kurlrequester below it. Aaron J. Seigo wrote: the Open With label is settable by using the KOpenWithDialog( const KUrl::List urls, const QString text, const QString value, QWidget *parent = 0 ); constructor, so it could be set to something sensible for this case. as forthe KUrlRequester ... is that so bad? it allows the user to define an arbitrary exec on disk, the ultimate last resort. Just tried KOpenWithDialog, and whilst you can set the text, and hide the checkboxes - the text entered in the url combo *must* be an application, not a desktop file. If you select a desktop file via the file selector you get the following error: 'XXX.desktop' not found, please type a valid program name. So, KOpenWithDialog would need updating to resolve this. On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: libs/taskmanager/taskactions.cpp, line 375 http://git.reviewboard.kde.org/r/103007/diff/1/?file=39955#file39955line375 dialogs must not be exec()'d as they *freeze* the entire UI.. in this case the whole plasma-desktop shell. i must be handled asynchronously. Craig Drummond wrote: Good point! Craig Drummond wrote: This is odd. I've tried running the dialog via show() and connecting to the okClicked() signal. However, the slot never gets called. If I change it to exec() the slot does get called... e.g. this does *not* work: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-show(); ...but this does: KApplicationSelectorDialog *dlg=new KApplicationSelectorDialog(i18n(XXX), 0L); connect(dlg, SIGNAL(okClicked()), this, SLOT(xxx())); dlg-exec(); (Ignore the obvious memory leak) - However, this seems to be a general issue with plasma dialogs. e.g. if you configure a colour for the clock dialog, plasma is frozen. Perhaps all plasma dialogs need to be run via another process? Aaron J. Seigo wrote: e.g. this does *not* work that is very, very odd. it would seem to imply that there is an event loop issue. there is no difference otherwise between exec() and show() (in fact, in fixing kcolorbutton i use show()). ah! perhaps i see the issue: LauncherProperties::run calls exec() on itself .. that creates an inner event loop around LauncherProperties. my guess is that trips it up. neither LauncherProperties nor KApplicationSelectorDialog should be exec()'d (Ignore the obvious memory leak) btw, easiest way to deal with this is to call dlg-setAttribute(Qt::WA_DeleteOnClose); this seems to be a general issue with plasma dialog we have this issue in very few places, and where it remains it is fixable .. i just fixed KColorButton right now in fact (thanks for pointing that one out) We are looking in different places of the code. The exec I was talking about was from the Show A Launcher action from the task menu. I'll change the LauncherProperties to not use exec... - Craig --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/#review7801 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/kapplicationselectordialog.h PRE-CREATION libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
On Nov. 1, 2011, 3:07 p.m., Aaron J. Seigo wrote: the wholesale copying of KOpenWithDialog is very unfortunate and needs to be avoided if at *all* possible. why is it copied instead of used directly? the really big issue is the exec()'ing of the dialog, however. it would also be nice to be able to access individual launcher configuration via the individual launchers' context menu. p.s. whitespace :) Craig Drummond wrote: I agree the copying is bad - but without changing kdelibs I knew of no other way. The reason it is copied is that in KOpenWithDialog you cannot remove the Open with label (or change its text), or the kurlrequester below it. Aaron J. Seigo wrote: the Open With label is settable by using the KOpenWithDialog( const KUrl::List urls, const QString text, const QString value, QWidget *parent = 0 ); constructor, so it could be set to something sensible for this case. as forthe KUrlRequester ... is that so bad? it allows the user to define an arbitrary exec on disk, the ultimate last resort. Craig Drummond wrote: Just tried KOpenWithDialog, and whilst you can set the text, and hide the checkboxes - the text entered in the url combo *must* be an application, not a desktop file. If you select a desktop file via the file selector you get the following error: 'XXX.desktop' not found, please type a valid program name. So, KOpenWithDialog would need updating to resolve this. indeed; i can fix that (though not today .. already at the end of my time alotments! h! why only 24 hours in a day?!) - Aaron J. --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/#review7801 --- On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Oct. 31, 2011, 8:42 p.m.) Review request for Plasma. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/kapplicationselectordialog.h PRE-CREATION libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION libs/taskmanager/launcherconfig.h PRE-CREATION libs/taskmanager/launcherconfig.cpp PRE-CREATION libs/taskmanager/launcherconfig.ui PRE-CREATION libs/taskmanager/launcherproperties.h PRE-CREATION libs/taskmanager/launcherproperties.cpp PRE-CREATION libs/taskmanager/launcherproperties.ui PRE-CREATION libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103007/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add fromTheme property to QIconItem in QtExtraComponents for theme icons in QML
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/#review7879 --- Ship it! let's go for it ;) - Marco Martin On Nov. 1, 2011, 6:43 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/ --- (Updated Nov. 1, 2011, 6:43 p.m.) Review request for KDE Runtime and Plasma. Description --- Hi, I found it very strange that QIcon::fromTheme was just working. Sadly in KDE's brand new PlasmaComponents it was simply impossible to use the system icons without using plasma to view the QML. This patch adds a property to QIconItem named: fromTheme which allows you to get the icons from the theme you currently use. This patch alone isn't helping a lot of people but i have a second patch ready for PlasmaComponents.Button to make use of this. Oke to commit? Diffs - plasma/declarativeimports/qtextracomponents/qiconitem.h 9972a98 plasma/declarativeimports/qtextracomponents/qiconitem.cpp d72381e Diff: http://git.reviewboard.kde.org/r/103009/diff/diff Testing --- Tested it with a bunch of different icons in and it all just seems to work fine. Thanks, Mark Gaiser ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Correct syntax for KRun URLs
KRun shoiuld be able to run an executable automatically without using the overloaded KRun::run. How should this be done? With arguments on the data string? ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Correct syntax for KRun URLs
On Wednesday, November 2, 2011 19:53:46 David Baron wrote: KRun shoiuld be able to run an executable automatically without using the overloaded KRun::run. How should this be done? With arguments on the data string? can you give a clear example of what you are wanting to have executed? it sounds like you are looking for KRun::runCommand, but without know precisely what you are trying to do, it is hard to know. -- 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 Development Frameworks 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: Correct syntax for KRun URLs
On Wednesday 05 Heshvan 5772 20:01:54 Aaron J. Seigo wrote: On Wednesday, November 2, 2011 19:53:46 David Baron wrote: KRun shoiuld be able to run an executable automatically without using the overloaded KRun::run. How should this be done? With arguments on the data string? can you give a clear example of what you are wanting to have executed? it sounds like you are looking for KRun::runCommand, but without know precisely what you are trying to do, it is hard to know. The KRun::runCommand is what I mean above. This works fine. I have some matches, some might refer to executable with arguments. Generally, new KRun(data, 0, ...) takes care of everything. Something like cmd:/... ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add fromTheme property to QIconItem in QtExtraComponents for theme icons in QML
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/#review7883 --- This review has been submitted with commit 67b24ecc2694e462a913284c16f3679a2e6851ab by Mark Gaiser to branch master. - Commit Hook On Nov. 1, 2011, 6:43 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/ --- (Updated Nov. 1, 2011, 6:43 p.m.) Review request for KDE Runtime and Plasma. Description --- Hi, I found it very strange that QIcon::fromTheme was just working. Sadly in KDE's brand new PlasmaComponents it was simply impossible to use the system icons without using plasma to view the QML. This patch adds a property to QIconItem named: fromTheme which allows you to get the icons from the theme you currently use. This patch alone isn't helping a lot of people but i have a second patch ready for PlasmaComponents.Button to make use of this. Oke to commit? Diffs - plasma/declarativeimports/qtextracomponents/qiconitem.h 9972a98 plasma/declarativeimports/qtextracomponents/qiconitem.cpp d72381e Diff: http://git.reviewboard.kde.org/r/103009/diff/diff Testing --- Tested it with a bunch of different icons in and it all just seems to work fine. Thanks, Mark Gaiser ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103007/ --- (Updated Nov. 2, 2011, 9:05 p.m.) Review request for Plasma. Changes --- 1. Don't use exec to run dialogs (except in ToggleLauncher action) 2. Replace ustom KAppltionSelectorDialog with KOpenWithDialog. KOpenWithDialog needs updating to handle .desktop files in url combo. 3. In setLauncherUrl(const KUrl) always save mapping. In a later patch will add setLauncherUrl(LauncherItem *) where the mappnig will not be saved. 4. Run astyle to fix any whitespace issues. Description --- 1. If fail to automatically find launcher, then prompt user to select from installed applications. 2. Add a config page, so that manualy set launchers may be adjusted. (Part of IconTasks' taskmanager changes) Diffs (updated) - libs/taskmanager/CMakeLists.txt 57f5f73 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/launcherconfig.h PRE-CREATION libs/taskmanager/launcherconfig.cpp PRE-CREATION libs/taskmanager/launcherconfig.ui PRE-CREATION libs/taskmanager/launcherproperties.h PRE-CREATION libs/taskmanager/launcherproperties.cpp PRE-CREATION libs/taskmanager/launcherproperties.ui PRE-CREATION libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103007/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add fromTheme property to QIconItem in QtExtraComponents for theme icons in QML
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/#review7884 --- plasma/declarativeimports/qtextracomponents/qiconitem.cpp http://git.reviewboard.kde.org/r/103009/#comment6814 Is there any specific reason why this isn't const QVariant icon? All used QVariant methods are const. - Christoph Feck On Nov. 1, 2011, 6:43 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/ --- (Updated Nov. 1, 2011, 6:43 p.m.) Review request for KDE Runtime and Plasma. Description --- Hi, I found it very strange that QIcon::fromTheme was just working. Sadly in KDE's brand new PlasmaComponents it was simply impossible to use the system icons without using plasma to view the QML. This patch adds a property to QIconItem named: fromTheme which allows you to get the icons from the theme you currently use. This patch alone isn't helping a lot of people but i have a second patch ready for PlasmaComponents.Button to make use of this. Oke to commit? Diffs - plasma/declarativeimports/qtextracomponents/qiconitem.h 9972a98 plasma/declarativeimports/qtextracomponents/qiconitem.cpp d72381e Diff: http://git.reviewboard.kde.org/r/103009/diff/diff Testing --- Tested it with a bunch of different icons in and it all just seems to work fine. Thanks, Mark Gaiser ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: Store launcher order
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103006/ --- (Updated Nov. 2, 2011, 10:04 p.m.) Review request for Plasma. Changes --- 1. Only store launchers in 1 container - not a hash for the launcher items, and a list for sort order. 2. In alphasorting strategy, use if/else and not terinary to make more readable 3. Add a 'forceGrouping' setting, so that applet can decide if items should *always* be grouped. Description --- 1. Store the order in which launchers are created 2. Add a new config option, separateLaunchers - so that applet can decide if launchers and tasks should be kept separate. (Part of IconTasks' taskmanager changes) Diffs (updated) - libs/taskmanager/launcheritem.cpp 20f0e7c libs/taskmanager/strategies/alphasortingstrategy.cpp 9ec1aca libs/taskmanager/strategies/desktopsortingstrategy.cpp 520fead libs/taskmanager/abstractgroupingstrategy.cpp 4ed424a libs/taskmanager/abstractsortingstrategy.cpp 390f6f0 libs/taskmanager/groupmanager.h acaa142 libs/taskmanager/groupmanager.cpp 6e7ffa7 libs/taskmanager/strategies/manualsortingstrategy.h 113faab libs/taskmanager/strategies/manualsortingstrategy.cpp 4409a6b libs/taskmanager/strategies/programgroupingstrategy.cpp 5c43d03 libs/taskmanager/taskgroup.h 53c2871 libs/taskmanager/taskgroup.cpp 49140ae libs/taskmanager/taskitem.h 5de8478 libs/taskmanager/taskitem.cpp 0a768e5 Diff: http://git.reviewboard.kde.org/r/103006/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add fromTheme property to QIconItem in QtExtraComponents for theme icons in QML
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/#review7885 --- This review has been submitted with commit 6a500cabcedebdb8b8cd342f2b3d2e0145cb86b5 by Mark Gaiser to branch master. - Commit Hook On Nov. 1, 2011, 6:43 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/ --- (Updated Nov. 1, 2011, 6:43 p.m.) Review request for KDE Runtime and Plasma. Description --- Hi, I found it very strange that QIcon::fromTheme was just working. Sadly in KDE's brand new PlasmaComponents it was simply impossible to use the system icons without using plasma to view the QML. This patch adds a property to QIconItem named: fromTheme which allows you to get the icons from the theme you currently use. This patch alone isn't helping a lot of people but i have a second patch ready for PlasmaComponents.Button to make use of this. Oke to commit? Diffs - plasma/declarativeimports/qtextracomponents/qiconitem.h 9972a98 plasma/declarativeimports/qtextracomponents/qiconitem.cpp d72381e Diff: http://git.reviewboard.kde.org/r/103009/diff/diff Testing --- Tested it with a bunch of different icons in and it all just seems to work fine. Thanks, Mark Gaiser ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Add fromTheme property to QIconItem in QtExtraComponents for theme icons in QML
On Nov. 2, 2011, 9:58 p.m., Christoph Feck wrote: plasma/declarativeimports/qtextracomponents/qiconitem.cpp, line 37 http://git.reviewboard.kde.org/r/103009/diff/2/?file=40071#file40071line37 Is there any specific reason why this isn't const QVariant icon? All used QVariant methods are const. fixed and pushed. - Mark --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/#review7884 --- On Nov. 1, 2011, 6:43 p.m., Mark Gaiser wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103009/ --- (Updated Nov. 1, 2011, 6:43 p.m.) Review request for KDE Runtime and Plasma. Description --- Hi, I found it very strange that QIcon::fromTheme was just working. Sadly in KDE's brand new PlasmaComponents it was simply impossible to use the system icons without using plasma to view the QML. This patch adds a property to QIconItem named: fromTheme which allows you to get the icons from the theme you currently use. This patch alone isn't helping a lot of people but i have a second patch ready for PlasmaComponents.Button to make use of this. Oke to commit? Diffs - plasma/declarativeimports/qtextracomponents/qiconitem.h 9972a98 plasma/declarativeimports/qtextracomponents/qiconitem.cpp d72381e Diff: http://git.reviewboard.kde.org/r/103009/diff/diff Testing --- Tested it with a bunch of different icons in and it all just seems to work fine. Thanks, Mark Gaiser ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: TaskManager: TaskAction changes from IconTasks
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103004/ --- (Updated Nov. 2, 2011, 10:46 p.m.) Review request for Plasma. Changes --- 1. Remove 'simple' changes - needs more thought. 2. Remove usage of custom properties. 3. Run astyle to fix whitespace issues. Description --- 1. Add NewInstance action to launch a new instance of an application. 2. Move toggle launcher action out of advanced menu. 3. Don't show application name in 'Show A Launcher' action, as might not know the real name at this time (e.g. no desktop file read). 4. Add application actions, to be shown at the top of the right-click menu. IconTasks uses this to show recent docuents, dock manager menu items, and unity items. 5. A ToolTipMenu class is created, so that tooltips may be give for menu items. This is so that IconTasks can display the full path of a documents in the recent documents menu. 6. Add option to have only basic window controls in menu. Diffs (updated) - libs/taskmanager/taskactions.h 2b5a641 libs/taskmanager/taskactions.cpp 0e6ba8e libs/taskmanager/taskactions_p.h 913966c Diff: http://git.reviewboard.kde.org/r/103004/diff/diff Testing --- Thanks, Craig Drummond ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request: Checking the return value of gps_open() as condition instead of checking m_gpsdata for prevent crash when no gpsd running
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103035/ --- Review request for Plasma. Description --- Checking the return value of gps_open() as condition instead of checking m_gpsdata for GPSD_API_MAJOR_VERSION =5, there should be prevent crash occurred when no gpsd running. Since m_gpsdata is instanced, there will be always consider gpsd found when no gpsd running, there should be check return value of gps_open() is not -1. This addresses bug 277036. http://bugs.kde.org/show_bug.cgi?id=277036 Diffs - plasma/generic/dataengines/geolocation/location_gps.cpp 709321b Diff: http://git.reviewboard.kde.org/r/103035/diff/diff Testing --- Testing this patch with gpsd 2.96(GPSD_API_MAJOR_VERSION =5) on trunk and openSUSE Factory, it works and some plasmoids(weather, weatherstation..) which with gpsd support are not crash anymore. However, unfortunately, I have no real gps device to test the function with gps device plugged, but I think this patch should be no broken somewhere with gps device plugged. Thanks, Max Lin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
