Review Request: Plasmate: Add KWin Scripting support to konsolepreviewer.

Antonis Tsiapaliokas kok3rs at gmail.com
Fri Aug 24 10:23:07 UTC 2012



> On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > mainwindow.cpp, line 261
> > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79947#file79947line261>
> >
> >     I don't really like this deviation in behavior, mainly because I don't see a reason for it.
> >     
> >     I think the behaviour of opening the konsole from the previewer is not ideal anyway, since that's a very unusual way to trigger a change in the main UI. The issue you're trying to address here is in the previewer, and access to the console should be in the main UI anyway (i.e. menu entry, shortcut, and possibly toolbar action).

Just for the KWin Scripts, we use the toolbar in order to toggle the konsolepreviewer.
For anything else, we use the previewer in order to toggle the konsolepreviewer.
Do you believe that we shouldn't use a previewer in order to toggle the konsolepreviewer?


> On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > konsole/konsolepreviewer.cpp, line 135
> > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79944#file79944line135>
> >
> >     Would be useful if the user was told what to do about it (i.e. either give the name of the expected file, or explain where you can set the name (and what it should be).

As it regard the ui, is a KMessageBox a solution or we should put it in the konsole's output?


> On Aug. 23, 2012, 10:19 p.m., Sebastian Kügler wrote:
> > mainwindow.cpp, line 870
> > <http://git.reviewboard.kde.org/r/106118/diff/1/?file=79947#file79947line870>
> >
> >     Seems like a toggle button (checkable) is needed here, instead of trying to come up with whacky new concepts of doing the exact same =).

For the KWin Scripts, we don't a previewer. So the real question is, should we hide previewer action or disabled(like a grey action which isn't clickable.)


- Antonis


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


On Aug. 22, 2012, 9:34 a.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106118/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2012, 9:34 a.m.)
> 
> 
> Review request for kwin, Plasma, Sebastian Kügler, Martin Gräßlin, and Giorgos Tsiapaliwkas.
> 
> 
> Description
> -------
> 
> Hello,
> 
> This patch is adding the konsolepreviewer support to the KWin Scripts.
> When the user, is pressing the "Execute" button, then the script is being called.
> Also on the toolbar, there is a new action with name "konsole". This action is visible only when we are inside to a KWin Script.
> All the other packages, have a previewer. So the "konsole" action is located inside the previewer.
> 
> 
> Diffs
> -----
> 
>   konsole/konsolepreviewer.h 8dd4369 
>   konsole/konsolepreviewer.cpp c4f9f8b 
>   main.cpp 298be51 
>   mainwindow.h 8005d26 
>   mainwindow.cpp b84da4a 
>   plasmateui.rc b51884d 
> 
> Diff: http://git.reviewboard.kde.org/r/106118/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> konsole
>   http://git.reviewboard.kde.org/r/106118/s/693/
> 
> 
> Thanks,
> 
> Antonis Tsiapaliokas
> 
>

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


More information about the Plasma-devel mailing list