Review Request: TaskManager: Prompt user when automatic task to launcher mathing fails.

[Craig Drummond]

> On Nov. 4, 2011, 11:29 a.m., Aaron J. Seigo wrote:
> > lots of coding style issues, though substance-wise it looks ok. a big concern i'm left with right now is that we end up with all of these patches in libtaskmanager and while it has more features, the overall quality of the code goes down due to the massive nature of the patches going in with very little testing combined with a decrease in readability due to less consistent coding style.
> > 
> > it also seems that in this patch we lose the ability to associate an executable with a launcher. that is a regression that needs addressing.
> > 
> > i also need to look at the config UI still, as well ...

Sorry for the coding issues - I thought I had run 'astyle', but perhaps I am mistaken. I *promise* to run 'astyle' when all the changes are checked in, and go through the files by hand to check the spacing. I agree that consistent style is a good idea.

As to not associating an executable with the launcher, this *was* mentioned even before I submitted the first diff. But anyway, yes this needs to be fixed. In fact the issue is already resolved, I just have not updated the patch - I dont have access to my git checkout at the moment.

I dont agree with the "very little testing", these changes are used in IconTasks - and that has been out for about couple of months.


> On Nov. 4, 2011, 11:29 a.m., Aaron J. Seigo wrote:
> > libs/taskmanager/launcherproperties.cpp, line 159
> > <http://git.reviewboard.kde.org/r/103007/diff/3/?file=40210#file40210line159>
> >
> >     this should be the correct values, not "-1U". NET::WM2WindowClass for the third and 0 for the first parameter..

OK, will do. Must admit this part was taken from the oxygen window decoration config.


- Craig


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103007/#review7940
-----------------------------------------------------------


On Nov. 3, 2011, 9:21 p.m., Craig Drummond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103007/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2011, 9:21 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 6ae36dc 
>   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 011c565 
>   libs/taskmanager/taskactions_p.h 407b2c9 
>   libs/taskmanager/taskitem.h 3c84678 
>   libs/taskmanager/taskitem.cpp e2c07d8 
> 
> Diff: http://git.reviewboard.kde.org/r/103007/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Craig Drummond
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20111104/bee3f47a/attachment.html>