D13178: Compress calls to configuration slots upon new connections

[Jacopo De Simoi]

jacopods updated this revision to Diff 36031.
jacopods added a comment.


  I used the direct connection as suggested.
  
  Today I learned about QOverload...

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13178?vs=35045&id=36031

REVISION DETAIL
  https://phabricator.kde.org/D13178

AFFECTED FILES
  kcms/keyboard/keyboard_daemon.cpp
  kcms/keyboard/keyboard_daemon.h

To: jacopods, PHID-PROJ-gqbvozptxawndyihp3hs, hein, broulik, drosca
Cc: mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D13178: Compress calls to configuration slots upon new connections

[Jacopo De Simoi]

jacopods added a comment.


  In D13178#270671 , @mart wrote:
  
  > are configureKeyboard configureMouse etc doing an immediate write to disk? 
(like KConfig::sync()
  
  
  Hey notmart, long time no see!
  
  The configureKeyboard() method is calling xmodmap (should the user need 
special configuration), so who knows what happens in there.

REVISION DETAIL
  https://phabricator.kde.org/D13178

To: jacopods, PHID-PROJ-gqbvozptxawndyihp3hs, hein, broulik, drosca
Cc: mart, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D13178: Compress calls to configuration slots upon new connections

[Jacopo De Simoi]

jacopods created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
jacopods requested review of this revision.

REVISION SUMMARY
  Some input devices register several copies of themselves upon connection 
(e.g. a mouse registers as a keyboard as well, or a keyboard registers twice as 
a keyboard).  Hence the keyboard/mouse configuration is called multiple times 
for no reason.
  

  
  This commit compresses the configuration calls using a singleshot timer.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D13178

AFFECTED FILES
  kcms/keyboard/keyboard_daemon.cpp
  kcms/keyboard/keyboard_daemon.h

To: jacopods
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Re: Review Request 128702: Use default weight rather than normal weight

[Jacopo De Simoi]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128702/
---

(Updated Sept. 15, 2016, 6:40 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 663c87bc6f3881091c04beb3ce447922945cf14e by David 
Edmundson on behalf of Jacopo De Simoi to branch master.


Repository: plasma-workspace


Description
---

Honor the choice of a user that has chosen a custom weight (e.g. light)
as default weight


Diffs
-

  applets/digital-clock/package/contents/ui/DigitalClock.qml 
48a310052eead844c2d7dbc22ee924a971733881 

Diff: https://git.reviewboard.kde.org/r/128702/diff/


Testing
---


Thanks,

Jacopo De Simoi

Review Request 128702: Use default weight rather than normal weight

[Jacopo De Simoi]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128702/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

Honor the choice of a user that has chosen a custom weight (e.g. light)
as default weight


Diffs
-

  applets/digital-clock/package/contents/ui/DigitalClock.qml 
48a310052eead844c2d7dbc22ee924a971733881 

Diff: https://git.reviewboard.kde.org/r/128702/diff/


Testing
---


Thanks,

Jacopo De Simoi

Re: RFC: Change Krunner's default key shortcut for Next

[Jacopo De Simoi]

> Currently the secondary shortcut is set to Alt + Shift + F2. I don't think
> any of us will miss this.

Actually this opens krunner with the content of the clipboard as the search 
term; quite handy sometimes. 

Is this change going to break this behavior?

__J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 112812: Use type description sort order in devicenotifier when non-removable devices are displayed

[Jacopo De Simoi]

> On Sept. 28, 2013, 1:59 a.m., Jacopo De Simoi wrote:
> > Let me have a look at the patch; I'll be able to do it tomorrow and commit 
> > it myself if it is ok. 
> > 
> > Thanks
> > 
> > __J 
> > Device notifier mantainer

Ok, even with the patch, within any given category (e.g. Storage Volume) 
devices are sorted in essentially random order (that is, the population order 
of the engine), so the patch is not addressing that issue.  On the other hand 
it surely helps a lot to have Network devices and Storage Volumes separated in 
the list when showing only non-removable devices.  
I still believe it is useful to have removable devices show up on top of other 
ones when showing all devices, so I am not convinced of changing the sort-order 
in this case. 

I would therefore change the sort order only if showing “non-removable devices 
only” and I am willing to commit this change if it is ok with the author. 

Still, a true solution is still to be found, so I'd rather not close the bug 
quite yet. 


- Jacopo De


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


On Sept. 19, 2013, 3:24 p.m., Benedikt Gollatz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112812/
> ---
> 
> (Updated Sept. 19, 2013, 3:24 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> ---
> 
> Use type description sort order in the devicenotifier applet when 
> non-removable devices are configured to be displayed. This avoids apparently 
> random sort order (by device engine population timestamp) and unneccessary 
> ListView sections. The problem becomes apparent if other types of devices 
> besides simple storage volumes are configured in /etc/fstab, like for example 
> network mounts.
> 
> Fixes bug #324459.
> 
> 
> This addresses bug 324459.
> http://bugs.kde.org/show_bug.cgi?id=324459
> 
> 
> Diffs
> -
> 
>   
> plasma/generic/applets/devicenotifier/package/contents/ui/devicenotifier.qml 
> 9b6132e 
> 
> Diff: http://git.reviewboard.kde.org/r/112812/diff/
> 
> 
> Testing
> ---
> 
> Works for me using KDE 4.10.5 packaged with Fedora 19.
> 
> 
> Thanks,
> 
> Benedikt Gollatz
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 112812: Use type description sort order in devicenotifier when non-removable devices are displayed

[Jacopo De Simoi]

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


Let me have a look at the patch; I'll be able to do it tomorrow and commit it 
myself if it is ok. 

Thanks

__J 
Device notifier mantainer

- Jacopo De Simoi


On Sept. 19, 2013, 3:24 p.m., Benedikt Gollatz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112812/
> ---
> 
> (Updated Sept. 19, 2013, 3:24 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> ---
> 
> Use type description sort order in the devicenotifier applet when 
> non-removable devices are configured to be displayed. This avoids apparently 
> random sort order (by device engine population timestamp) and unneccessary 
> ListView sections. The problem becomes apparent if other types of devices 
> besides simple storage volumes are configured in /etc/fstab, like for example 
> network mounts.
> 
> Fixes bug #324459.
> 
> 
> This addresses bug 324459.
> http://bugs.kde.org/show_bug.cgi?id=324459
> 
> 
> Diffs
> -
> 
>   
> plasma/generic/applets/devicenotifier/package/contents/ui/devicenotifier.qml 
> 9b6132e 
> 
> Diff: http://git.reviewboard.kde.org/r/112812/diff/
> 
> 
> Testing
> ---
> 
> Works for me using KDE 4.10.5 packaged with Fedora 19.
> 
> 
> Thanks,
> 
> Benedikt Gollatz
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Mono icons in systray

[Jacopo De Simoi]

> This is indeed a considerable overhead; perhaps then one could implement
> shape-rendering:crispEdges rendering in QtSvg; actually if the svg routines
> are shared with webkit, it could be that they are already implemented…

ok, this might be pure nonsense; I checked out the specs of what crispEdges 
does and it is not what I was expecting…

Best,
 __J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Mono icons in systray

[Jacopo De Simoi]

On Tuesday 11 December 2012 21:37:13 Marco Martin wrote:
> On Tuesday 11 December 2012, Jacopo De Simoi wrote:
> > recalling some recent post on icon fonts, I wonder if switching to an icon
> > font for monochrome plasma icons would provide us with a solution; in my
> 
> > opinion there are quite a number of advantages:
> using a font would basically raise the entry barrier of doing a new icon to
> plus infinite or so..

This seems to be a very sensible objection; I believe that inkscape can 
produce svg fonts, which can then be imported in fontforge and tweaked with 
hinting infos there. 

This is indeed a considerable overhead; perhaps then one could implement 
shape-rendering:crispEdges rendering in QtSvg; actually if the svg routines 
are shared with webkit, it could be that they are already implemented…

> 
> having hinting there would be very cool (the shape deformation to fit the
> grid, not the horrible color bleeding antialiasing abomination)

Allright, subpixel hinting is definitely not your favourite feature; I got 
your point :D 

Best,   
   __J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Mono icons in systray

[Jacopo De Simoi]

On Tuesday 11 December 2012 12:13:11 you wrote:
> A Terça, 11 de Dezembro de 2012 10:06:36 você escreveu:
> > Dear all,
> > 
> >   I noticed how the new air theme (which is just beautiful, btw) makes the
> > 
> > mono icons in systray quite low-constrast; this is especially apparent
> > with
> > compositing turned off.
> 
> they seam about the same here ...

I am not sure, it looks like the panel is lighter than it used to be

> agrea to some extent that we should get rid of the iner gradient in the
> icons. the border i sthere for contrast issue againts random
> backgroundsNote fonts suck in that way and we have to make the (imo)
> overdone blured background.

Yes, I understand the issue with random backgrounds, but I am referring to the 
non-composited case, where no transparency is taken into account. At least 
this case, and the composited case with the default wallpaper should be nicely 
contrasted. 

> > - hinting: fonts provide hinting informations for pixel-snapping on-screen
> > rendering at small sizes (which are typically the ones used in systray).
> > This does not make the icon pixel perfect, but possibly quite close to it.
> > Manually hinting a font is quite painful, but after all we do not have too
> > many icons to take care of…
> 
> again do you know any font designer willing to do them?
> and what is the adgantage work wise in relation to svg's? you can do the
> same in svg's creting multiple svg's for multiple rendering sizes

btw, why are we not using pngs in desktoptheme icons?

> > - subpixel antialiasing:  subpixel antialiasing (for those who like it)
> > would possibly make curves smoother
> 
> so mono But with color pixels on the side right

It's a matter of taste; besides, if you see color fringing you should consider 
tweaking your subpixel filter (check out infinality) 

> > - coloring: icons can be colored in such a way to optimize contrast, this
> > makes borders around shapes not necessary and help saving precious pixels
> 
> but ofcourse the problem is that we dont have controll over the background
> color since that is tranparent

That is why we should provide a way to set coloring of icons; after all if one 
chooses the same color for background and foreground, then his desktop is 
broken, but it is a user fault; otoh if the user cannot choose the foreground 
color… then it is not that clear. 

> Since it would just duplicate work, with no visible gain imo (with QT5
> around the corner coloring an element in scenegraph is rader trivial,
> shaders for the win)... 

That's excellent news!

> On the making the try icons mono yeah expect that soon...
\me drools
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Mono icons in systray

[Jacopo De Simoi]

Dear all, 
 
  I noticed how the new air theme (which is just beautiful, btw) makes the 
mono icons in systray quite low-constrast; this is especially apparent with 
compositing turned off.   
I started thinking about a sensible solution which would not require 
rebuilding all mono icons every time we change the default plasma theme; 
recalling some recent post on icon fonts, I wonder if switching to an icon 
font for monochrome plasma icons would provide us with a solution; in my 
opinion there are quite a number of advantages:

- the new air theme (and the whole design trend these days) is considerably 
“flatter” than the old ones; perhaps *really* flat mono icons could fit better 
than the current ones, with all these gradients and borders. Besides, one 
could in principle render fonts with a gradient and a border, as it is already 
done for the keyboard layout applet and the plasma digital clock. 

- hinting: fonts provide hinting informations for pixel-snapping on-screen 
rendering at small sizes (which are typically the ones used in systray). This 
does not make the icon pixel perfect, but possibly quite close to it.  
Manually hinting a font is quite painful, but after all we do not have too 
many icons to take care of…

- subpixel antialiasing:  subpixel antialiasing (for those who like it) would 
possibly make curves smoother 

- coloring: icons can be colored in such a way to optimize contrast, this 
makes borders around shapes not necessary and help saving precious pixels 

Of course there are a few disadvantages:

- things get more complicated with composite icons, (e.g. volume or battery), 
so we should cook out some reasonable way to define things such as svg groups 
for fonts

- Learn to use a new tool (e.g. fontforge) to design such icons (altough we 
could obtain some help from Vernon Adams, who is designing the Oxygen font)

My proposal is to create some sort of plasma icon service, which would create 
QIcons of the right size™ from a special font; this would be totally 
transparent to applets, exactly as it is done now with svg icons embedded in 
plasma themes. 

So I am asking, what are your opinions on this? Does this make sense to you? 
Do you think is it worth working in this direction? (I'm heading for 4.11, of 
course) 

Thanks, 
  Best

  __J

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: make the DeviceNotifier Plasmoid scale with high-res screens

[Jacopo De Simoi]

> On Nov. 26, 2012, 9:35 a.m., Marco Martin wrote:
> > This is really an issue that needs addressing, thanks for taking time for 
> > it.
> > 
> > i'm very in favor of removing all those hardcoded sizes from the code.
> > 
> > however i see two problems with that approach
> > 
> > 1) icons are cleanly painted only when they have a size that is the "right" 
> > one (16,22,32,48,64,128,512 exported in qml with theme.smallIconSize, 
> > smallMediumIconSize etc)  for big sizes doesn't matter if the pixmap ends 
> > up being scaled, but small icons really look horrible when scaled (and in 
> > the new code there is no check the size ends up being one of those).
> > 
> > 2) it's anyways quite arbitrary, so i can easily see each plasmoid using 
> > more or less its own logic, with the end result looking a bit like a 
> > patchwork.
> > 
> > the"proper" solution i think is to export in Theme the configurable 
> > KIconLoader sizes, (Desktop, Toolbar, SmallIcons etc)   and those can be 
> > controlled by a kcm, so will be properly set on higher resolution displays
> 
> Marco Martin wrote:
> just added the bindings:
> use
> theme.iconSizes.dialog for most icons
> theme.iconSizes.toolbar for unmount
> theme.iconSizes.small for all that is normally 16x16
> 
> 
> so the icon settings in systemsettings will affect those icons
> 
> Michael Zanetti wrote:
> Hey Marco,
> 
> I'm not sure if this is really a good idea. Using KIconLoader::IconSize() 
> is a good idea for toolbars and such. However, in a plasma layout I don't 
> think its a good idea to let the user change the size of the icons. It can 
> and will mess up the layout. For example I use the same size for "Toolbar" 
> and "Dialog" because I like big toolbars. I certainly don't want to the 
> unmount icon to be that huge in the devicemanager plasmoid because if that...
> 
> regarding the scaling: I was thinking that Plasma uses SVG icons... 
> Reading throught the QIconItem code I see now that it doesn't. In that case 
> its obviously not good to freely scale it around. Maybe it should be replaced 
> by PlasmaCore.SvgItem?
> 
> Marco Martin wrote:
> what is important is that the sizes of the icons are something that is
> a) consistent everywhere (not having each applet decide with its criteria)
> b) be something that can depend from dpi, those being settable are fine, 
> since will be the same as things around application
> 
> now, indeed can be not a good idea to have the unmount icon as toolbar, 
> since the size can be messed up (so keeping a size relative to the size used 
> to the main icon, that i think dialog is ok)
> 
> This i think makes the case for exposing a currently internal component 
> that i'm using in buttons and toolbuttons called IconLoader
> the behavior is:
> * use an svg if it does exist in the theme: requiring to have a complete 
> svg theme (and with enough quality) is neither feasible nor desiderable
> * fallback to a qicon if the svg is not found
> * always paint in the nearest (smaller) kiconloader standard size (even 
> for svg, for consistency in layouts and because they aren't as scalable as 
> they seem, if their shape edges are not grid aligned they look horrible)
> * will probably also have to directly support the on mouse over highlight 
> animation or people will keep using the horrible PlasmaWidgets.IconWidget
>
> 
> Jacopo De Simoi wrote:
> Hi Michael,
>   
>   indeed I can well see your point. Using a fraction of the size of the 
> dialog icon might be so-so (in fact the ratio 22/32 is quite unique in our 
> icons, it would basically be good just for the current setting.).The best 
> looking (although horrible code-wise) thing to do would probably be to cook 
> up a map from the size of a device icon to the size of an action icon 
> (without worrying about rounding issues); for instance we would have: 32 -> 
> 22; 48 -> 32; 64 -> 48. Same thing for the emblems, we would need 32 -> 16; 
> 48 -> 22; 64 -> 32.
> What do you think?  Marco? 
>
> 
> Michael Zanetti wrote:
> What IMHO would be really needed is to extend KIconloader:
> 
> a) deprecate the enum KIconLoader::StdSizes asap as it shouln't be used 
> ever again
> b) create a replacement for it in the form of: "int 
> KIconLoader::standardSize(Small | Medium | Large | whatnot)" that returns a 
> pixelsizes that is calculated using the DPIs
> 
> Then this could be used where

Re: Review Request: make the DeviceNotifier Plasmoid scale with high-res screens

[Jacopo De Simoi]

> On Nov. 26, 2012, 9:35 a.m., Marco Martin wrote:
> > This is really an issue that needs addressing, thanks for taking time for 
> > it.
> > 
> > i'm very in favor of removing all those hardcoded sizes from the code.
> > 
> > however i see two problems with that approach
> > 
> > 1) icons are cleanly painted only when they have a size that is the "right" 
> > one (16,22,32,48,64,128,512 exported in qml with theme.smallIconSize, 
> > smallMediumIconSize etc)  for big sizes doesn't matter if the pixmap ends 
> > up being scaled, but small icons really look horrible when scaled (and in 
> > the new code there is no check the size ends up being one of those).
> > 
> > 2) it's anyways quite arbitrary, so i can easily see each plasmoid using 
> > more or less its own logic, with the end result looking a bit like a 
> > patchwork.
> > 
> > the"proper" solution i think is to export in Theme the configurable 
> > KIconLoader sizes, (Desktop, Toolbar, SmallIcons etc)   and those can be 
> > controlled by a kcm, so will be properly set on higher resolution displays
> 
> Marco Martin wrote:
> just added the bindings:
> use
> theme.iconSizes.dialog for most icons
> theme.iconSizes.toolbar for unmount
> theme.iconSizes.small for all that is normally 16x16
> 
> 
> so the icon settings in systemsettings will affect those icons
> 
> Michael Zanetti wrote:
> Hey Marco,
> 
> I'm not sure if this is really a good idea. Using KIconLoader::IconSize() 
> is a good idea for toolbars and such. However, in a plasma layout I don't 
> think its a good idea to let the user change the size of the icons. It can 
> and will mess up the layout. For example I use the same size for "Toolbar" 
> and "Dialog" because I like big toolbars. I certainly don't want to the 
> unmount icon to be that huge in the devicemanager plasmoid because if that...
> 
> regarding the scaling: I was thinking that Plasma uses SVG icons... 
> Reading throught the QIconItem code I see now that it doesn't. In that case 
> its obviously not good to freely scale it around. Maybe it should be replaced 
> by PlasmaCore.SvgItem?
> 
> Marco Martin wrote:
> what is important is that the sizes of the icons are something that is
> a) consistent everywhere (not having each applet decide with its criteria)
> b) be something that can depend from dpi, those being settable are fine, 
> since will be the same as things around application
> 
> now, indeed can be not a good idea to have the unmount icon as toolbar, 
> since the size can be messed up (so keeping a size relative to the size used 
> to the main icon, that i think dialog is ok)
> 
> This i think makes the case for exposing a currently internal component 
> that i'm using in buttons and toolbuttons called IconLoader
> the behavior is:
> * use an svg if it does exist in the theme: requiring to have a complete 
> svg theme (and with enough quality) is neither feasible nor desiderable
> * fallback to a qicon if the svg is not found
> * always paint in the nearest (smaller) kiconloader standard size (even 
> for svg, for consistency in layouts and because they aren't as scalable as 
> they seem, if their shape edges are not grid aligned they look horrible)
> * will probably also have to directly support the on mouse over highlight 
> animation or people will keep using the horrible PlasmaWidgets.IconWidget
>

Hi Michael,
  
  indeed I can well see your point. Using a fraction of the size of the dialog 
icon might be so-so (in fact the ratio 22/32 is quite unique in our icons, it 
would basically be good just for the current setting.).The best looking 
(although horrible code-wise) thing to do would probably be to cook up a map 
from the size of a device icon to the size of an action icon (without worrying 
about rounding issues); for instance we would have: 32 -> 22; 48 -> 32; 64 -> 
48. Same thing for the emblems, we would need 32 -> 16; 48 -> 22; 64 -> 32.
What do you think?  Marco? 


- Jacopo De


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


On Nov. 23, 2012, 10:11 p.m., Michael Zanetti wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107428/
> ---
> 
> (Updated Nov. 23, 2012, 10:11 p.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Description
> ---
> 
> This adjustst the DeviceNotifier Plasmoid to scale nicely with high 
> resolution screens.
> 
> The patch changes also one label from wordWrapping to clipping behavior 
> because otherwise it messes up the layout.
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/package/

Re: Review Request: make the DeviceNotifier Plasmoid scale with high-res screens

[Jacopo De Simoi]

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


Thanks for addressing this issue, it is a pity that qml makes it so hard to 
handle such things nicely.
I hope to be able to test this during the weekend; the diff looks reasonable, 
though. 
Thanks

__J
--Device notifier mantainer

- Jacopo De Simoi


On Nov. 23, 2012, 10:11 p.m., Michael Zanetti wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107428/
> ---
> 
> (Updated Nov. 23, 2012, 10:11 p.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Description
> ---
> 
> This adjustst the DeviceNotifier Plasmoid to scale nicely with high 
> resolution screens.
> 
> The patch changes also one label from wordWrapping to clipping behavior 
> because otherwise it messes up the layout.
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/package/contents/ui/ActionItem.qml 
> 3087a07 
>   plasma/generic/applets/devicenotifier/package/contents/ui/DeviceItem.qml 
> 1fab0ef 
>   
> plasma/generic/applets/devicenotifier/package/contents/ui/devicenotifier.qml 
> f8728d0 
> 
> Diff: http://git.reviewboard.kde.org/r/107428/diff/
> 
> 
> Testing
> ---
> 
> Tested on High res screen at full DPI and at regular DPI values
> 
> 
> Thanks,
> 
> Michael Zanetti
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Device notifier: show mounted device and path

[Jacopo De Simoi]

On Friday 12 October 2012 15:56:18 Sebastian Kügler wrote:
> On Friday, October 12, 2012 13:42:35 Lamarque Vieira Souza wrote:
> > This tooltip looks really odd and out of place this way.:/
> > The transparency effect does not look good here. I would like to know how
> > to disable it too, there is this bug against the QML shutdown dialog with
> > the same problem: https://bugs.kde.org/show_bug.cgi?id=303934
> 
> No, I'm not talking about the translucency, I'm talking about having a
> tooltip on top of a popup, that's weird (and IMO a showstopper).

Well, the notifier already shows several tooltips as it is now.
- one appearing when hovering over the device icon
- one appearing when hovering over the mount/unmount action
- one appearing when hovering over the capacity bar

People were complaining about icons not being informative enough, hence we had 
to add tooltips here and there :/

Any suggestion to improve the current state is most welcome :)

__J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Device notifier: show mounted device and path

[Jacopo De Simoi]

> On Oct. 8, 2012, 9:52 a.m., Aaron J. Seigo wrote:
> > any application which expects the user to access files on disk but does not 
> > provide a clear representation of mounted / removable devices is broken. 
> > there's no point in degrading our own primary UI for such fixable 
> > brokenness. can you provide a (short :) list of any such broken 
> > applications which are high-profile / in common use?
> 
> Aaron J. Seigo wrote:
> ah, and i should also add that tooltips on items in popup windows as it 
> leads to a matryoshka doll effect that is most inelegant visually, so we try 
> to avoid such things unless there is a very good reason for them.
> 
> Jonathan Marten wrote:
> I think too that nested popups/tooltips look inelegant, but since it was 
> Jacopo's suggestion I though it may be at least worth trying it out...
> 
> In my experience hardly any non-KDE applications are able to show or 
> control the mounting of removable devices - most of them stick to the 
> hopeless Gnome file selector, and not all of them can be made to work well 
> with KDE integration.  Certainly in my daily use neither mozilla, 
> libreoffice/openoffice, gimp or googleearth identify removable devices.  
> Inkscape is the only major application that at least tries.  They are indeed 
> broken (by design), but they are unlikely to be fixed before the heat death 
> of the universe.

My point was rather to stuff that information in some --already present-- 
tooltip, such as the one appearing hovering over the capacity bar or the device 
icon. 
But yes this is indeed still visually unpleasant. Besides, I agree with Aaron 
about creating some “Technical info” pane with all this information, which 
could be triggered by context menu.
I do not want to clutter the ui any further. It's already pretty full.

Cheers 


- Jacopo De


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


On Oct. 8, 2012, 9:31 a.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106755/
> ---
> 
> (Updated Oct. 8, 2012, 9:31 a.m.)
> 
> 
> Review request for KDE Base Apps and Plasma.
> 
> 
> Description
> ---
> 
> If a removable device is mounted using the Plasma device notifier, there is 
> no indication of what the Unix name of the device is or where it is mounted.  
> This information may be useful to the user for (a) accessing the mounted 
> device from non-KDE applications, or (b) troubleshooting mounting or 
> unmounting problems.
> 
> The attached patch shows this information when the device is hovered over, 
> just above the "N actions for this device" text.  Depending on whether or not 
> the device is mounted, there are three possibilities that can be shown here:
> 
>   /dev/XXX   when not mounted
>   /dev/XXX mounted on /media/when mounted
>   /dev/XXX mounted   if mounted but the mount point is not 
> available
> 
> Please be gentle, this is my first QML patch :-)
> 
> 
> This addresses bug 196939.
> http://bugs.kde.org/show_bug.cgi?id=196939
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/package/contents/ui/DeviceItem.qml 
> 396de2c 
> 
> Diff: http://git.reviewboard.kde.org/r/106755/diff/
> 
> 
> Testing
> ---
> 
> Built kde-workspace with this change, observed operation and display of 
> device notifier with a selection of removable devices.
> 
> 
> Screenshots
> ---
> 
> Device notifier with mounted device
>   http://git.reviewboard.kde.org/r/106755/s/756/
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Device notifier: show mounted device and path

[Jacopo De Simoi]

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


I do not want the notifier to show this information in the device view; indeed, 
for devices with a label, the mount point is /media/, so the information 
about the mount point would look like quite redundant.
Perhaps you could show it in a tooltip, instead; that would be acceptable imho

In any case, the “trouble with unmounting” usecase should be resolved in some 
other way (e.g. by making use of lsof in solid to identify blocking apps), so I 
am not super-sure that the user should be given such specific information as 
/dev/. 
 

- Jacopo De Simoi


On Oct. 7, 2012, 2:55 p.m., Jonathan Marten wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106755/
> ---
> 
> (Updated Oct. 7, 2012, 2:55 p.m.)
> 
> 
> Review request for KDE Base Apps and Plasma.
> 
> 
> Description
> ---
> 
> If a removable device is mounted using the Plasma device notifier, there is 
> no indication of what the Unix name of the device is or where it is mounted.  
> This information may be useful to the user for (a) accessing the mounted 
> device from non-KDE applications, or (b) troubleshooting mounting or 
> unmounting problems.
> 
> The attached patch shows this information when the device is hovered over, 
> just above the "N actions for this device" text.  Depending on whether or not 
> the device is mounted, there are three possibilities that can be shown here:
> 
>   /dev/XXX   when not mounted
>   /dev/XXX mounted on /media/when mounted
>   /dev/XXX mounted   if mounted but the mount point is not 
> available
> 
> Please be gentle, this is my first QML patch :-)
> 
> 
> This addresses bug 196939.
> http://bugs.kde.org/show_bug.cgi?id=196939
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/package/contents/ui/DeviceItem.qml 
> 2a9b3f7 
> 
> Diff: http://git.reviewboard.kde.org/r/106755/diff/
> 
> 
> Testing
> ---
> 
> Built kde-workspace with this change, observed operation and display of 
> device notifier with a selection of removable devices.
> 
> 
> Screenshots
> ---
> 
> Device notifier with mounted device
>   http://git.reviewboard.kde.org/r/106755/s/756/
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [RFC] Trigger KRunner on top screenedge

[Jacopo De Simoi]

On Wednesday 03 October 2012 13:44:06 Martin Gräßlin wrote:
> Hi all,
> 
> just wanted some feedback on an idea I got today. What about binding the
> showing of KRunner on hitting the top screen edge?
> 
> Given that KRunner slides in from the top edge it would end up in a neat
> functionality, throw the cursor against the edge and down comes KRunner.
> Maybe even centered at the cursor position?
> 
> I don't think that the top screen edge would conflict with anything else.
> When using for quick-maximization of maximized windows the action would not
> be triggered anyway and for grabbing a maximized window to move it away it
> would not matter either as you have to push against the edge.
> 
> So opinions? Should I add support for that in KWin? It's pretty trivial :-)

I am using a virtualbox on a macbook as my work/devel machine (yes, that 
hurts) and the Mac menu bar pops up every time I hit the top border with the 
mouse. I find it most irritating, since the menu bar is totally useless for 
me, but on the other hand I sometimes try to hit the top border to trigger 
krunner because I feel like “something” should happen. 

In short: i have mixed feelings about it; sorting out exceptions could prove 
to be quite difficult indeed. Let's add the feature and see how it goes. ;)

__J



___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Use Product instead of description for device names

[Jacopo De Simoi]

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

Ship it!


Ship It!

- Jacopo De Simoi


On Sept. 29, 2012, 8:21 p.m., Alex Fiestas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106637/
> ---
> 
> (Updated Sept. 29, 2012, 8:21 p.m.)
> 
> 
> Review request for Plasma and Viranch Mehta.
> 
> 
> Description
> ---
> 
> The description is useful for when the device is not hotpluggable/removeable, 
> for example to show:
> 96.3 GiB Hard Drive
> 15.1 GiB Hard Drive
> 
> instead of two identical labels.
> 
> But when it comes to removable/hotpluggable we want to show the Product to be 
> able to show:
> 
> Nokia N9
> Nexus 7
> 
> Instead of 
> Portable Media Player
> Portable Media Player
> 
> The difference can be shown also in the attached screenshots.
> 
> 
> Diffs
> -
> 
>   
> plasma/generic/applets/devicenotifier/package/contents/ui/devicenotifier.qml 
> b23df45 
> 
> Diff: http://git.reviewboard.kde.org/r/106637/diff/
> 
> 
> Testing
> ---
> 
> 
> Screenshots
> ---
> 
> before
>   http://git.reviewboard.kde.org/r/106637/s/743/
> after
>   http://git.reviewboard.kde.org/r/106637/s/744/
> 
> 
> Thanks,
> 
> Alex Fiestas
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: better handling of Removable property for volumes in soliddevice engine

[Jacopo De Simoi]

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


Just my 2 cents: 
  This piece of API does not seem to belong to the dataengine.
I would rather consider inclusion in the solid API; I vaguely recall having 
read (or written) some similar code somewhere else, so
it would definitely make sense to put it in the libs. 

Best Regards

- Jacopo De Simoi


On July 5, 2012, 6:44 a.m., Andriy Gapon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105432/
> ---
> 
> (Updated July 5, 2012, 6:44 a.m.)
> 
> 
> Review request for Plasma and Solid.
> 
> 
> Description
> ---
> 
> Currently the code in SolidDeviceEngine::populateDeviceData sets volume's 
> Removable property in straightforward fashion:
> - tries to cast volume's (immediate) parent device to Solid::StorageDrive
> - if that succeeds, checks isHotpluggable() and isRemovable() properties of 
> the parent
> The problem is that there could be intermediate devices in solid device 
> hierarchy between the volume and its Solid::StorageDrive ancestor, for 
> example partition-type devices.
> 
> The proposed code walks up the hierarchy to try harder to find 
> Solid::StorageDrive ancestor of a given device.
> 
> BTW, I think that getAncestorOfType() helper function introduced in the 
> proposed change can be useful for other code too, so it might make sense to 
> nominate it for inclusion into Solid API.
> 
> 
> Diffs
> -
> 
>   plasma/generic/dataengines/soliddevice/soliddeviceengine.cpp 608da25 
> 
> Diff: http://git.reviewboard.kde.org/r/105432/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andriy Gapon
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Plasma Applet Testing for 4.9

[Jacopo De Simoi]

On Monday 21 May 2012 00:37:07 Viranch Mehta wrote:
> On Thu, May 17, 2012 at 3:58 AM, David Edmundson  > wrote:
> >  - a list of all the new QML-based applets (by the time of the first beta)
> >  
> >(afaik, nowplaying, battery, locklogout, activitymanager.. but
> > 
> > there are so many more random branches about, and I don't know the
> > status of these)
> 
> As for the status, device notifier was shipped in last release, 
Indeed, but many features have not yet been tested extensively, so please do 
not neglect it in your tests even if it has been out there for a while :)
Also, any usability feedback is very much welcome

__J
-- 
KDE Plasma Device Notifier mantainer


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Avoid multiple map - engine connections

[Jacopo De Simoi]

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

(Updated May 21, 2012, 1:51 p.m.)


Review request for Plasma and Aaron J. Seigo.


Description
---

This fixes some multiple connection issues; up to now we connect the map to the 
engine once for each device that is added, hence when some device changes its 
state, it triggers the corresponding signal in the map, which in turn triggers 
multiple times  the corresponding slot in the engine.
 
This seems to have been like that from ages. Was there a reason for that?


Diffs
-

  plasma/generic/dataengines/soliddevice/devicesignalmapmanager.cpp 4549cd4 

Diff: http://git.reviewboard.kde.org/r/105003/diff/


Testing
---

basic testing: everything works as expected.


Thanks,

Jacopo De Simoi

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Avoid multiple map - engine connections

[Jacopo De Simoi]

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

Review request for Plasma and Aaron J. Seigo.


Description
---

This fixes some multiple connection issues; up to now we connect the map to the 
engine once for each device that is added, hence when some device changes its 
state, it triggers the corresponding signal in the map, which in turn triggers 
multiple times  the corresponding slot in the engine.
 
This seems to have been like that from ages. Was there a reason for that?


Diffs
-

  plasma/generic/dataengines/soliddevice/devicesignalmapmanager.cpp 4549cd4 

Diff: http://git.reviewboard.kde.org/r/105003/diff/


Testing
---

basic testing: everything works as expected.


Thanks,

Jacopo De Simoi

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add option to not show popup when devices are added

[Jacopo De Simoi]

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


I am not sure if I actually would allow to block the device notifier from 
popping up altogheter; 
If a user doesn't want the notifier to popup for a specific device she can 
simply hide it by right-clicking on the device item.

- Jacopo De Simoi


On Oct. 11, 2011, 1:30 p.m., Simon Persson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102825/
> ---
> 
> (Updated Oct. 11, 2011, 1:30 p.m.)
> 
> 
> Review request for Plasma, Giulio Camuffo and Jacopo De Simoi.
> 
> 
> Description
> ---
> 
> Use case:
> User 1 frequently connects her phone to her laptop to charge the battery, she 
> has no intention of accessing files on the phone and thinks the device 
> notifier popping up (even for a few seconds) is distracting. When accessing 
> files on an external drive she usually uses an already running instance of 
> her file manager and again would prefer the notifier to not pop up. She keeps 
> the notifier in her panel for the case of easily watching dvd's and 
> downloading photos from a camera memory card. In those cases she thinks it's 
> enough that the icon appears with an exclamation mark in her systray, she can 
> find it there.
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/configurationpage.ui a916254 
>   plasma/generic/applets/devicenotifier/devicenotifier.h 9f3af73 
>   plasma/generic/applets/devicenotifier/devicenotifier.cpp b9dfce5 
> 
> Diff: http://git.reviewboard.kde.org/r/102825/diff/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Simon Persson
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Device notifier popup did not hide after the intended timeout of 7.5 s.

[Jacopo De Simoi]

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


Sorry for the late reply.
First of all notice that there is a complete qml rewrite of the device notifier 
which could make it for 4.8.
I still don't know if it will be the case, so, I'll review your patch in any 
case.

I kind of remember that the call to activate() was indeed useful, but I have to 
recollect my memories, which unfortunately doesn't happen right now.
I'll have a more in-depth look.
Please bear with me a bit more; spare time is very scarce in these days.

- Jacopo De Simoi


On Oct. 11, 2011, 10:18 a.m., Simon Persson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102824/
> ---
> 
> (Updated Oct. 11, 2011, 10:18 a.m.)
> 
> 
> Review request for Plasma, Giulio Camuffo and Jacopo De Simoi.
> 
> 
> Description
> ---
> 
> This is my first fix for plasma, maybe there's a reason to emit 
> plasma::applet::activated() that I don't know. Removing it fixes the problem 
> and I can't see any regression.
> I also needed to remove activated() signal from NotifierDialog, I consider 
> this one to be unnecessary since there is an eventfilter that on 
> QEvent::GraphicsSceneHoverMove triggers the popup to be open for 7.5 s more. 
> In other words, the popup will now close after 7.5 s of inactive mouse if it 
> was shown as a result of plugging in a device... which I guess was the 
> intended behavior.
> 
> Couldn't actually find any open bug report for this. (!)
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/notifierdialog.cpp dff38d9 
>   plasma/generic/applets/devicenotifier/notifierdialog.h 8a03fc5 
>   plasma/generic/applets/devicenotifier/devicenotifier.cpp b9dfce5 
> 
> Diff: http://git.reviewboard.kde.org/r/102824/diff/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Simon Persson
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Make the activities runner available for single runner use

[Jacopo De Simoi]

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

Review request for Plasma and Aaron J. Seigo.


Summary
---

Enables the activities runner for single runner use; 
allows for fast activity switching using an appropriate shortcut


Diffs
-

  plasma/generic/runners/activities/activityrunner.cpp 
4df11268fc0f03abd79b972aaa90c40b6b27b9cf 
  plasma/generic/runners/activities/plasma-runner-activityrunner.desktop 
036b389bcfc5237bf74d6a3f11c0bab9e5979270 

Diff: http://git.reviewboard.kde.org/r/101144/diff


Testing
---

Compiled and it works


Thanks,

Jacopo De

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Expand last plugged device in Device Notifier applet

[Jacopo De Simoi]

What happens when a device with multiple partitions is plugged in?
Afaics all of them are expanded, which I'm not sure it is really what I'd like 
to happen,
because the second partition is likely to be hidden below the first one…

> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100904/
> ---
> 
> Review request for Plasma and Aaron J. Seigo.
> 
> 
> Summary
> ---
> 
> This patch ensures that when a device is plugged, it appears expanded in the 
> Device Notifier applet. This means only one click is necessary to trigger the 
> device action.
> 
> 
> Diffs
> -
> 
>   plasma/generic/applets/devicenotifier/devicenotifier.cpp 796a10b 
>   plasma/generic/applets/devicenotifier/notifierdialog.h a1cc81d 
>   plasma/generic/applets/devicenotifier/notifierdialog.cpp 84a0b9b 
> 
> Diff: http://git.reviewboard.kde.org/r/100904/diff
> 
> 
> Testing
> ---
> 
> - With only one device => device appears expanded.
> - With a collapsed device, plugged a new device => only new device appears 
> expanded.
> 
> 
> Thanks,
> 
> Aurélien
> 
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: about commit 83ea19670eff0febff516380fb59a65a1d7667a9: use Qt::Popup in tasks applet

[Jacopo De Simoi]

> On Tuesday, March 8, 2011, Jacopo De Simoi wrote:
> > This strategy is quite debatable, but apparently has been there forever
> > (<4.5) Can we workaround this fact?
> 
> i don't know; to me it really sounds like something that needs to be fixed in 
> Qt.
here we go, merge request 

http://qt.gitorious.org/qt/qt/merge_requests/1135
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

about commit 83ea19670eff0febff516380fb59a65a1d7667a9: use Qt::Popup in tasks applet

[Jacopo De Simoi]

Hi plasma ppl,

The issue with pixmap initialization I referred to in my previous thread hits 
most notably popups in the tasks applet. Even if good care has been put in 
avoiding this to happen (with the fix described in the previous thread), the 
commit in 
the subject breaks this attempts, since, for some reason, the Qt pixmap 
creation strategy is the same for windows with 
Qt::Popup or with Qt::WA_NoSystemBackground. 
That is, even it WA_NoSystemBackground is unset, but Qt::Popup is set, then the 
Sliding Popup effect will break. 

This strategy is quite debatable, but apparently has been there forever (<4.5) 
Can we workaround this fact? reverting this commit should do it, is it worth it?
Any other solutions? the glitch is quite horrible (at least on my machine)

-
As a side note, this commit introduces a regression: when a popup is opened, 
clicking 
on another task item makes the popup retract, but somehow focus is still kept 
by the
same task item; clicking anywhere in the tasks applet (but on the appropriate 
task item) makes the popup
open and retract until one clicks on the appropriate task item is clicked 
again. 
-

Thanks
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Take care of your WA_TranslucentBackground flag

[Jacopo De Simoi]

 A few days ago I started investigating about some visual glitches happening 
with the sliding popup effect,
which are caused by a "bizarre" initial pixmap initialization strategy.

Basically what happens is that under some conditions (most notably when the Qt 
widget window has 
the Qt::WA_NoSystemBackground flag set) the window pixmap is initialized on 
creation with a copy
of the portion of screen covered by the window geometry. This can cause trouble 
when (for instance with 
the sliding popup effect) the window is translated around the screen; you can 
see for a split second 
(depending on your machine this time can be longer) see parts of your screen 
replicated and moving around.
Quite a bad glitch indeed. 

Long story short: WA_NoSystemBackground breaks the sliding popup effects, but 
it is otoh necessary in a non-composited setting. 

The issue is that setting WA_TranslucentBackground automatically sets 
WA_NoSystemBackground, so one should take care of unsetting the latter flag 
after setting the former, if compositing is enabled and setting/unsetting it if 
compositing is disabled/enabled.
Most notably this applies to krunner, to the activity manager and other 
settings that I may have not noticed. 
It's a quite straightforward fix; I'll fix those two when I have time (if 
somebody wants to fix them now, plz do!). If you notice
that your piece of code might be impacted by this issue, please have a look and 
fix it. 
I'm most often on irc if you need help
Thx 

 __J 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [patch] enabling autorun in devicenotifier applet

[Jacopo De Simoi]

> On Wednesday, January 5, 2011, Jacopo De Simoi wrote:
> > Was there a specific reason for patching the notifier?
> 
> yep: Maarten, who wanted, the feature appear on irc and i offered a few 
> places 
> he could do this, and he picked the easiest and shortest path. the itch he 
> was 
> scratching is this: he's put together a "media" computer using Plasma Destop 
> which he has locked down quite a bit. one feature he needed was for media 
> plugged in to "just work", e.g. pop in a DVD and xine starts up.
> 
> he now has this working with his customized distro (i forget which one he 
> started with as a basis.. fedora? anyways.. he's been testing his changes via 
> patching and building new packages and installing them that way...)
> 
> i asked him to email his patch here so we could go from where he left off to 
> something we can include in upstream :)
> 
> if nothing else, this provides a reasonable proof of concept, which shows 
> that 
> there is both value to the idea and that it works in even this basic form.
> 

Ok, excellent! 
I can help with moving the code to a more appropriate place (if help is needed 
of course!)
 
Cheers!
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [patch] enabling autorun in devicenotifier applet

[Jacopo De Simoi]

> Hi,
> 
> I've made this patch with some directions from aseigo.
> 
> I wanted some kind of autorun settings for specific devices dependant on the 
> media of optical drives; so i made a patch to recognize X-KDE-Autorun in 
> .desktop file action entries.
> 
> this patch was for 4.5.90 and it builds. couldn't test it's functionality.
> 
> it can be tested by adding X-KDE-Autorun=true in a solid/actions/*.desktop 
> file 
> (in the "open" action eg)

I am not sure this should belong to the device notifier; in fact imho it should 
rather be in the device automounter daemon or in solid. 
Was there a specific reason for patching the notifier?
Thanks!

 __J

Plasma device notifier mantainer
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: KDE/kdebase/workspace/plasma/generic/applets/devicenotifier

[Jacopo De Simoi]

> On Wednesday 14 July 2010 20:10:42 Jacopo De Simoi wrote:
> > SVN commit 1149977 by jacopods:
> > 
> > First attempt to get rid of the capacity bar in the device notifier: use a
> > Plasma::Meter with tooltips. Screenshot:
> > http://imagebin.ca/view/ccJ3dD.html
> 
> No longer displays amount of free space --> regression.
> Avoidable tooltips -> Usability problem.

I agree, suggestions?

> Bug you mentioned -> Qt bug (fixed in 4.7).

Indeed, since Qt 4.7 would probably be required for KDE SC 4.6, I agree that 
I could in principle leave it as-is. 
 
> What are the actual improvements then?
I'm just trying to experiment early in the release cycle to see if I can get 
some good ideas;
The point is to make sure to have a consistent look; right now the look of the 
capacitybar 
depends on the qt widget style, which doesn't sound exactly right to me.
I mean... I can live with it, but I'd like to come up with something more, 
well… elegant :)
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: KDE/kdebase/workspace/plasma/generic/applets/devicenotifier

[Jacopo De Simoi]

> On July 14, 2010, Jacopo De Simoi wrote:
> > SVN commit 1149977 by jacopods:
> > 
> > First attempt to get rid of the capacity bar in the device notifier: use a
> > Plasma::Meter with tooltips. Screenshot:
> > http://imagebin.ca/view/ccJ3dD.html
> 
> the screenshot looks nice; but i'm curious: what were the issues with 
> KCapacityBar in this case? 

I had in mind issues such as the following br:

https://bugs.kde.org/show_bug.cgi?id=231609
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

KDE/kdebase/workspace/plasma/generic/applets/devicenotifier

[Jacopo De Simoi]

SVN commit 1149977 by jacopods:

First attempt to get rid of the capacity bar in the device notifier: use a 
Plasma::Meter with tooltips.
Screenshot: http://imagebin.ca/view/ccJ3dD.html

Feedback is more than welcome
CCMAIL:plasma-devel@kde.org


 M  +17 -17deviceitem.cpp  
 M  +3 -2  deviceitem.h  


--- 
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/deviceitem.cpp
 #1149976:1149977
@@ -33,7 +33,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 //Plasma
@@ -42,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -107,16 +107,16 @@
 m_descriptionLabel->setOpacity(0);
 updateColors();
 
-KCapacityBar *capacityBarWidget = new 
KCapacityBar(KCapacityBar::DrawTextInline);
-m_capacityBar = new QGraphicsProxyWidget(this);
-m_capacityBar->setWidget(capacityBarWidget);
-capacityBarWidget->setAttribute(Qt::WA_TranslucentBackground);
-capacityBarWidget->setContinuous(true);
-m_capacityBar->setAcceptHoverEvents(false);
-m_capacityBar->setOpacity(0);
+
+m_freeSpaceBar = new Plasma::Meter();
+m_freeSpaceBar->setMeterType(Plasma::Meter::BarMeterHorizontal);
+m_freeSpaceBar->setLabelAlignment(0, Qt::AlignCenter);
+m_freeSpaceBar->setOpacity(0);
+m_freeSpaceBar->setMaximumHeight(12);
+
 info_layout->addItem(m_nameLabel);
 info_layout->addItem(m_descriptionLabel);
-info_layout->addItem(m_capacityBar);
+info_layout->addItem(m_freeSpaceBar);
 
 m_leftActionIcon = new Plasma::IconWidget(this);
 
m_leftActionIcon->setMaximumSize(m_leftActionIcon->sizeFromIconSize(LEFTACTION_SIZE));
@@ -317,9 +317,9 @@
 }
 }
 
-const bool barVisible = m_capacityBar->isVisible();
-m_capacityBar->setVisible(m_mounted && allowsCapacityBar());
-if (!barVisible && m_capacityBar->isVisible()) {
+const bool barVisible = m_freeSpaceBar->isVisible();
+m_freeSpaceBar->setVisible(m_mounted && allowsCapacityBar());
+if (!barVisible && m_freeSpaceBar->isVisible()) {
 // work around for a QGraphicsLayout bug when used with proxy widgets
 m_mainLayout->invalidate();
 }
@@ -360,7 +360,7 @@
 m_barFade = 
Plasma::Animator::create(Plasma::Animator::FadeAnimation, this);
 
 m_labelFade->setTargetWidget(m_descriptionLabel);
-m_barFade->setTargetWidget(m_capacityBar);
+m_barFade->setTargetWidget(m_freeSpaceBar);
 
 m_labelFade->setProperty("targetOpacity", 0);
 m_barFade->setProperty("targetOpacity", 0);
@@ -378,15 +378,15 @@
 void DeviceItem::setHoverDisplayOpacity(qreal opacity)
 {
 m_descriptionLabel->setOpacity(opacity);
-m_capacityBar->setOpacity(opacity);
+m_freeSpaceBar->setOpacity(opacity);
 }
 
 void DeviceItem::setFreeSpace(qulonglong freeSpace, qulonglong size)
 {
 qulonglong usedSpace = size - freeSpace;
-KCapacityBar *capacityBarWidget = 
static_cast(m_capacityBar->widget());
-capacityBarWidget->setText(i18nc("@info:status Free disk space", "%1 
free", KGlobal::locale()->formatByteSize(freeSpace)));
-capacityBarWidget->setValue(size > 0 ? (usedSpace * 100) / size : 0);
+
+m_freeSpaceBar->setToolTip(i18nc("@info:status Free disk space", "%1 
free", KGlobal::locale()->formatByteSize(freeSpace)));
+m_freeSpaceBar->setValue(size > 0 ? (usedSpace * 100) / size : 0);
 }
 
 void DeviceItem::leftActionClicked()
--- 
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/deviceitem.h 
#1149976:1149977
@@ -33,6 +33,7 @@
 class IconWidget;
 class BusyWidget;
 class Label;
+class Meter;
 }
 
 namespace Notifier
@@ -332,8 +333,8 @@
 ///The label that draws the description of the device
 Plasma::Label *m_descriptionLabel;
 
-///The proxy hosting the capacity bar
-QGraphicsProxyWidget *m_capacityBar;
+///The meter that draws free space for the device
+Plasma::Meter *m_freeSpaceBar;
 
 ///The busy widget
 Plasma::BusyWidget *m_busyWidget;
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Change device notifier tooltips to respect plasma tooltip style

[Jacopo De Simoi]

> On May 22, 2010, Jacopo De Simoi wrote:
> > I'm playing around with that now, and here's what I got so far...
> > how do you like it?
> 
> it puts a third size of icon in the dialog, and it isn't really aligned with 
> anything visually.

Yep, I was trying to make it look like an extender in fact, but evidently I 
miserably failed :D

> my recommendation would be to just ditch the icon altogether. it doesn't 
> really provide any content. center the text at the top. 

I'll try and see if this convinces me

> only show the 
> separators between devices if there is more than one type.

As I said in my other post to this thread, I think that the widget 
"separator+category name" should be visually regarded as one thing, hence I'd 
rather not make the distinction; /either/ no separator and no category name if 
just one category is present /or/ both visible… 
In my opinion it is better to have them always visible… 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Change device notifier tooltips to respect plasma tooltip style

[Jacopo De Simoi]

> > Like one more font face
> > and not aligned with anything either.
> 
> the font should be the same as the top header, so there shouldn't be another 
> font face. as for alignment, iirc they are right aligned.

You recall correctly, but it has been changed since :)
Right aligning with (un)mount buttons always visible gives a weird feeling, as 
if the buttons were part of 
a column which name is given by the category name. 
The current visualization (i.e. left aligned category name below a separator) 
recalls the usual category view in other parts of the workspace (e.g. system 
settings), although admittedly it could be made a dedicated widget in 4.6.

The category title should be aligned with either the device icon or the 
itemBackground; the former looks decent with no hovered item, the latter looks 
better in the other case. In any way, since most icons are not square, the 
category title is pretty much /never/ aligned with the icon (it tries to do so 
now), so I'll just go and left align it with the itemBackground.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Change device notifier tooltips to respect plasma tooltip style

[Jacopo De Simoi]

> > you mean, to make the icon smaller?
> 
> Yes, it takes *way* too much space. And, what's the point? The user
> will learn very soon what this widget does, so then it's kinda
> unnecessary. There are a lot of other plasmoids out there. Some of
> them do have a header, some of them don't. I have a crazy idea.
> make it like in calendar popup, which has a small stripe with a tiny
> icon, just for uniform look. Is that possible without architectural
> changes (extender)? Or remove it.

I'm playing around with that now, and here's what I got so far...
how do you like it?

http://imagebin.ca/view/aNTmIVf9.html

__J

P.S. if you show up in irc, plz ping me (wilder), i didn't quite get your last 
comment
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Change device notifier tooltips to respect plasma tooltip style

[Jacopo De Simoi]

> Well, as far as device notifier goes, there's a couple more things about it 
> that I'd like to see fixed as well. 

Here I am, the device notifier mantainer, at your service :)

> This one about tooltips sounds nice, but then it kinda duplicates what's in 
> the real device list, and there seems to be a limitation on the tooltip size 
> (is there?) Really don't know what's the best one, maybe a very compact list 
> would work.
I wouldn't duplicate the content of the applet, really, but a mockup would 
indeed be appreciated

> Then, there's one more point: kill those separators (it's time to get rid of 
> them in a lot of places, I think :),
the separators subdivide different categories of devices (e.g. camera, optical 
media, etc etc), they are quite needed imho; maybe we could get rid of just the 
first one, though I'm not sure

> and squeeze the "Available devices" with the icon vertically. 
you mean, to make the icon smaller?

> There's too much margin space on both sides. 
I agree, I am going to do that as soon as I resolve some other layout issues

> All pure aesthetics, but then, it's screen space and clutter. If you don't 
> mind, I'll hack on this, and deliver a prototype of device list popup for the 
> applet.
Popup means tooltip?

Thanks
  __J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Routing (un)mounting error notifications via knotify; part II: dataengine

[Jacopo De Simoi]

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

Review request for Plasma, Aaron Seigo, Marco Martin, and Olivier Goffart.


Summary
---

This patch provides a dataengine which collects and exposes (un)mounting error 
notifications; the plasma devicenotifier will spawn an instance of this 
dataengine to tell knotify that it will display such notifications itself. 


Diffs
-

  trunk/KDE/kdebase/workspace/plasma/generic/dataengines/CMakeLists.txt 1123205 
  
trunk/KDE/kdebase/workspace/plasma/generic/dataengines/devicenotifications/CMakeLists.txt
 PRE-CREATION 
  
trunk/KDE/kdebase/workspace/plasma/generic/dataengines/devicenotifications/devicenotificationsengine.h
 PRE-CREATION 
  
trunk/KDE/kdebase/workspace/plasma/generic/dataengines/devicenotifications/devicenotificationsengine.cpp
 PRE-CREATION 
  
trunk/KDE/kdebase/workspace/plasma/generic/dataengines/devicenotifications/org.kde.DeviceNotifications.xml
 PRE-CREATION 
  
trunk/KDE/kdebase/workspace/plasma/generic/dataengines/devicenotifications/plasma-dataengine-devicenotifications.desktop
 PRE-CREATION 

Diff: http://reviewboard.kde.org/r/3896/diff


Testing
---

This patch depends on the corresponding one for knotify;

With that patch it works as expected.


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

KDE/kdebase/workspace/plasma/generic/applets/devicenotifier

[Jacopo De Simoi]

SVN commit 1113686 by jacopods:

Show error messages inside the applet instead of using Plasma::MessageBox
As soon as a patch in solid is accepted, the notifier will be able to react to 
errors triggered by other applications, hence errors will be shown also when 
triggered from other applications (e.g. dolphin). 

Feedback is welcome!

CCMAIL:plasma-devel@kde.org


 M  +7 -1  devicenotifier.cpp  
 M  +69 -9 notifierdialog.cpp  
 M  +20 -0 notifierdialog.h  


--- 
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.cpp
 #1113685:1113686
@@ -158,6 +158,7 @@
 } else {
 m_dialog->collapseDevices();
 }
+changeNotifierIcon();
 }
 
 void DeviceNotifier::dataUpdated(const QString &udi, Plasma::DataEngine::Data 
data)
@@ -445,7 +446,12 @@
 
 void DeviceNotifier::showErrorMessage(const QString &message)
 {
-showMessage(KIcon("dialog-error"), message, ButtonOk);
+m_dialog->showStatusBarMessage(message);
+emit activate();
+showPopup(NOTIFICATION_TIMEOUT);
+changeNotifierIcon("dialog-error");
+update();
+QTimer::singleShot(NOTIFICATION_TIMEOUT, m_dialog, 
SLOT(resetNotifierIcon()));
 }
 
 bool DeviceNotifier::areThereHiddenDevices()
--- 
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/notifierdialog.cpp
 #1113685:1113686
@@ -462,8 +462,8 @@
 m_widget->installEventFilter(this);
 m_widget->setFocusPolicy(Qt::ClickFocus);
 
-QGraphicsLinearLayout *l_layout = new QGraphicsLinearLayout(Qt::Vertical, 
m_widget);
-l_layout->setSpacing(0);
+m_mainLayout = new QGraphicsLinearLayout(Qt::Vertical, m_widget);
+m_mainLayout->setSpacing(0);
 
 Plasma::IconWidget *icon = new Plasma::IconWidget(m_widget);
 icon->setIcon(KIcon("emblem-mounted"));
@@ -486,7 +486,7 @@
 QGraphicsWidget *titleWidget = new QGraphicsWidget();
 titleWidget->setLayout(l_layout2);
 
-l_layout->addItem(titleWidget);
+m_mainLayout->addItem(titleWidget);
 
 m_devicesScrollWidget = new Plasma::ScrollWidget(m_widget);
 QGraphicsWidget *devicesWidget = new 
QGraphicsWidget(m_devicesScrollWidget);
@@ -496,7 +496,7 @@
 m_deviceLayout->setContentsMargins(0, 0, 0, 8);
 devicesWidget->setLayout(m_deviceLayout);
 
-l_layout->addItem(m_devicesScrollWidget);
+m_mainLayout->addItem(m_devicesScrollWidget);
 
 m_itemBackground = new Plasma::ItemBackground(devicesWidget);
 m_selectedItemBackground = new Plasma::ItemBackground(devicesWidget);
@@ -506,17 +506,72 @@
 connect(m_itemBackground, SIGNAL(animationStep(qreal)), this, 
SLOT(itemBackgroundMoving(qreal)));
 connect(m_selectedItemBackground, SIGNAL(animationStep(qreal)), this, 
SLOT(itemBackgroundMoving(qreal)));
 
+m_statusWidget = new QGraphicsWidget();
+QGraphicsLinearLayout *statusLayout = new 
QGraphicsLinearLayout(Qt::Vertical, m_statusWidget);
+
+m_statusWidget->setLayout(statusLayout);
+Plasma::Separator *statusSeparator = new Plasma::Separator();
+statusSeparator->setOrientation(Qt::Horizontal);
+statusSeparator->setSizePolicy(QSizePolicy::MinimumExpanding, 
QSizePolicy::Fixed);
+statusLayout->addItem(statusSeparator);
+
+m_statusText = new Plasma::Label();
+m_statusText->setSizePolicy(QSizePolicy::MinimumExpanding, 
QSizePolicy::Minimum);
+
+Plasma::IconWidget *closeButton = new Plasma::IconWidget();
+closeButton->setSvg("widgets/configuration-icons", "close");
+
closeButton->setMaximumSize(closeButton->sizeFromIconSize(KIconLoader::SizeSmall/2));
+closeButton->setMinimumSize(closeButton->maximumSize());
+
+connect(closeButton, SIGNAL(clicked()), this, SLOT(dismissStatusBar()));
+
+QGraphicsLinearLayout *labelLayout = new 
QGraphicsLinearLayout(Qt::Horizontal);
+Plasma::IconWidget *infoIcon = new Plasma::IconWidget();
+infoIcon->setIcon("preferences-desktop-notification");
+
infoIcon->setMaximumSize(infoIcon->sizeFromIconSize(KIconLoader::SizeSmall));
+
infoIcon->setMinimumSize(infoIcon->sizeFromIconSize(KIconLoader::SizeSmall));
+
+labelLayout->addItem(infoIcon);
+labelLayout->setAlignment(infoIcon, Qt::AlignVCenter);
+labelLayout->addItem(m_statusText);
+labelLayout->setAlignment(m_statusText, Qt::AlignVCenter);
+labelLayout->addItem(closeButton);
+labelLayout->setAlignment(closeButton, Qt::AlignVCenter);
+
+statusLayout->addItem(labelLayout);
+
 devicesWidget->adjustSize();
 updateMainLabelText();
 
-m_widget->setLayout(l_layout);
+m_widget->setLayout(m_mainLayout);
 m_widget->setMinimumSize(250, 300);
 }
 
+void NotifierDialog::dismissStatusBar()
+{
+m_statusWidget->hide();
+m_mainLayout->removeItem(m_statusWidget);
+}
+
+void NotifierDialog::showStatusBarMessage(const QString & message)
+{
+m_statusText->setText(message);
+m_statusText->adjustSize();
+m_statusWidget->adjustSize();
+m_mainLayout->addItem(m_statusWidget);
+m_statusWidget->show();
+}
+
 void NotifierDial

Re: Running queries via dbus

[Jacopo De Simoi]

I just got a quite crazy idea; it might work though. 
Since we are making the thread stall by waiting for the async reply we could as 
well check that while we get the pending reply the context is still valid and 
bail out discarding the reply otherwise. If this can be coded in a simple way I 
guess it's a good solution for preventing stacking up of threads in krunner
Aaron ,what do you think about this?

Thanks 

 __J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Running queries via dbus

[Jacopo De Simoi]

> On Tuesday 23 March 2010 21:55:08 Alex Merry wrote:
> > Instead, you should use QDBusConnection::asyncCall.  This can be used in
> > exactly the same way (implicit casting FTW), and casting to a QDBusReply
> > will cause the calling thread to block as you would expect.  But,
> > importantly, it WON'T cause the main thread to block.

In fact the runner thread will block; and since we limit ourselves to run only 
so many threads in krunner, this
implies that a user running an older version of amarok will still experience 
long (~25s) freezes. if he launches enough queries 
(just typing a long name is sufficient as we launch roughly as many queries as 
keystrokes (which is bad anyways)
krunner won't have any thread slots available and will become useless, if not 
block the ui.

Is there any way to check for the version of amarok and avoid doing that query 
if it is <= the current release?

Thanks

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Running queries via dbus

[Jacopo De Simoi]

> CC'ing plasma-devel, since this is an important point regarding runners that 
> use D-Bus.  Please keep any discussion on this issue at plasma-devel.
> 
> On Tuesday 23 March 2010 17:26:54 Alex Merry wrote:
> > On Monday 22 March 2010 22:05:26 Jacopo De Simoi wrote:
> > > > In particular, Meta::Track has float as the return type of bpm().
> > > > Perhaps it would be sensible to make this a qreal?  Also, score()
> > > > should probably also return qreal (rather than double) for
> > > > consistency.

did you commit this fix as well? 

> I'll commit this change to audioplayercontrol to trunk.  Should I also 
> backport it?
I'd say yes please, if it fixes any freeze related issue, it would be great to 
have it in branch as well.
Notice that this would possibly fix bug 203668; you might want to CCBUG that

Thanks a lot, I'll try right now if the fix on the amarok side works

__J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: KCM icons, krunner and consistency

[Jacopo De Simoi]

> On Sunday 07 March 2010 15:55:05 Jacopo De Simoi wrote:
> > Browsing .desktop files of KCMs you can notice that there is no common
> > convention; some of them use "xxx Configuration", some others "Configure
> > xxx", others "xxx settings", others "xxx". It would be nice to fix a rule
> > and stick to it, what do you think?
> 
> What about imposing just 'xxx' in the desktop file and adding "Configuration" 
> or similar by "hand"? We'd have to ask the i18n teams if this does create any 
> problems though...

That's precisely what I wanted to avoid by providing the "configure" emblem on 
the icon in krunner.
That should be a good enough (and i18n-safe) approach to the problem. Moreover, 
since icons would be drawn by the oxygen people and not written by a random 
developer, we could have better control on the fact that kcm icons should not 
contain by themselves any "configuration" hint. And I would make this policy as 
strong as the one on the .desktop files comments; after all we are talking 
about the same issue, only with a different visual representation.

Regards
__J

> 
> Bye,
> -Riccardo
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: n+1st report about arrow keys in krunner

[Jacopo De Simoi]

> On 8 March 2010 01:42, Aaron J. Seigo  wrote:
> 
> > this reminds me about jokes about the difference between theory and
> > practice.
> >
> > if we remove the history then we'll get to deal with tons of bug reports
> > about
> > how you can't do "alt+f2, up arrow, enter" which is a very, very common use
> > pattern.
> >
> > Who says you can't do that? What you *shouldn't* do in this context is
> opening a different list when pressing Up and when pressing Down. You still
> can open the list when you press Up.
> 
>  that theory of "history isn't used" is, in the face of real world practice,
> 
> > dead wrong.
> >
> 
> I dind't mean "history isn't used", I meant "history is not separate from
> bookmarks", which is what the *practical* experiments performed on browser
> history showed.
> 
> 
> 
> >
> > > Every search function
> >
> > krunner is not a search tool. it can be used to search for things, but
> > that's
> > not the extant of its definition.
> >
> 
> If it can be used to search for things, it's a search tool - whether the
> designer wants it this way or not. It's the user who decides what the tool
> does for them, not the programmer. If it has a search function, someone will
> use it for search.

Krunner is a keyboard-based (minicli) interface to the workspace, and it is 
designed as such.
This also definitely implies that it can be used as a search facility, it's not 
its main purpose but it does not necessarily conflict with the primary goal; in 
fact I find it a very useful tool.

> 
> If the search tool conflicts with the tool's primary goal, either remove the
> search functionality or make it not conflicting. There's a standard
> interaction for "history+search from a predefined list". I say don't
> reinvent the wheel, use the existing expected interface - or turn it into
> something other than history+search.

The issue here is imo the fact that the examples of 'history+search from a 
predefined list' you are taking  do not refer to a history of queries (such as 
krunner history), but rather to a history of results (such as firefox history).
This complicate things quite a bit, since there's not anymore a one to one 
correspondence between history elements and results, but each history elements 
provides in principle a number of different results. 
I am not able to see how they should apply to our case, unless we are willing 
to implement krunner history as an history of "recent results" rather than 
"recent queries". Whether we should or not, I guess it's open to discussion...

__J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: 4.5 polishment: krunner (under_the_hood)

[Jacopo De Simoi]

> Am Montag 08 März 2010 22:43:33 schrieb Jacopo De Simoi:
> > > Am Montag 08 März 2010 01:28:23 schrieb Aaron J. Seigo:
> > > > On March 7, 2010, Lukas Appelhans wrote:
> > > > > I can give a patch if needed of course... it's just a matter of
> > > > > porting it from the app-runner to main KRunner...
> > 
> > I'd love to see the patch!
> > 
> > Further, I've a question, since I don't really have any real experience
> > with developing with nepomuk: how does this sorting thing would actually
> > /perform/ in krunner? I can see it working ok in a "menu" context, where
> > queries are pretty much hardcoded, but in krunner we need ideally an
> > instant reaction to user queries; I'm afraid that querying nepomuk at
> > (basically) each keystroke could potentially take some time. Please tell
> > me I'm wrong :)
> Well Nepomuk should be fast, at least if it uses a suitable backend (Virtuoso 
> :))...
Does each query still need a dbus roundtrip?  That can be quite slow in fact :/
 
> At the moment, we just compute a score out of the click-count/click-time/last-
> score... not for each keystroke, but for each program...

Yes, but new programs may come up with each keystroke, and this would lead to 
a computation for each keystroke, at least if I understand correctly

> 
> I will check whether it makes sense or is easily doable to change to every 
> keystroke or not :)
> 
> Lukas
> > 
> > __J
> > 
> > > > yes, it makes zero sense to make things specific to a runner. there is
> > > > no guarantee that other runners won't return applications (and in fact
> > > > the Shell runner already does) and non-application matches are no less
> > > > relevant/interesting. so a generalized patch would be good.
> > > 
> > > Ok, I will work on that once I have time (next 2 weeks will be quite busy
> > > ;))...
> > > 
> > > Lukas
> > > ___
> > > Plasma-devel mailing list
> > > Plasma-devel@kde.org
> > > https://mail.kde.org/mailman/listinfo/plasma-devel
> > 
> > ___
> > Plasma-devel mailing list
> > Plasma-devel@kde.org
> > https://mail.kde.org/mailman/listinfo/plasma-devel
> 
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: 4.5 polishment: krunner (under_the_hood)

[Jacopo De Simoi]

> Am Montag 08 März 2010 01:28:23 schrieb Aaron J. Seigo:
> > On March 7, 2010, Lukas Appelhans wrote:
> > > I can give a patch if needed of course... it's just a matter of porting
> > > it from the app-runner to main KRunner...
> > 

I'd love to see the patch!

Further, I've a question, since I don't really have any real experience with 
developing with nepomuk: 
how does this sorting thing would actually /perform/ in krunner? I can see it 
working ok in a "menu" context, where queries are pretty much hardcoded, but in 
krunner we need ideally an instant reaction to user queries; I'm afraid that 
querying nepomuk at (basically) each keystroke could potentially take some 
time. 
Please tell me I'm wrong :)

__J


> > yes, it makes zero sense to make things specific to a runner. there is no
> > guarantee that other runners won't return applications (and in fact the
> > Shell runner already does) and non-application matches are no less
> > relevant/interesting. so a generalized patch would be good.
> Ok, I will work on that once I have time (next 2 weeks will be quite busy 
> ;))...
> 
> Lukas
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

KCM icons, krunner and consistency

[Jacopo De Simoi]

Dear devs, 
 in view of our commitment to polish for 4.5, I'd like to raise a small issue 
regarding systemsettings and more specifically KCM icons and .desktop files

At the moment icons for kcms can be shown in a few different settings, in 
systemsettings, in the "configure" menu of applets or applications, in 
krunner...
In the first three cases kcms are shown in a setting which is clearly a 
"configuration" context. As such, the presence of a sort of "configure" hint in 
the icon (i.e. a wrench or the systemsettings icon) is quite redundant. In fact 
at the moment, I can see ~35 KCMs listed under systemsettings, and only 3-5 
(depending on how you count them) items have a wrench in their icons. 

My proposal is to use some new icons for those items, either by removing the 
overlay or by making new ones (Nuno, it's up to you :)) in such a way to create 
uniformity among icons in systemsettings and avoid redundancy.

On the other hand, a "configure emblem" would be most useful in krunner; I 
would like to overlay all icons related to kcms with such an emblem, in such a 
way that it is clear that it's a configuration module and not an application 
(i've got a few too many bug reports complaining about that.. try querying 
"kontact" with krunner and guess which one is the KCM and which is the 
application)
Oxygen team, could you do that?

and by the way, this leads me to .desktop files!
Browsing .desktop files of KCMs you can notice that there is no common 
convention; some of them use "xxx Configuration", some others "Configure xxx", 
others "xxx settings", others "xxx".
It would be nice to fix a rule and stick to it, what do you think?

Thanks a lot.
Regards

 __J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

4.5 polishment: krunner (under_the_hood)

[Jacopo De Simoi]

Hi there, this is the first part of my ideas for polishing krunner for 4.5. 
This thread will focus mostly about under_the_hood (i.e. not UI) changes, sooo 
here are my ideas:

* Better sorting: at the moment krunner remembers which items the user runs 
frequently and kicks them up in the results list.
While this works well in principle, it has some drawbacks, let me give a few 
examples:
I run quite often konsole via krunner, and now, whenever I look for "system", 
konsole comes up first, because it belongs to the "system" category and it has 
an enormous boost due to the extremely high run count.
Moreover I might look for xterm and write "term..", but still konsole would win.
Also, I run gwenview pretty often with krunner and it always shows up first 
when looking for "view", same thing for emacs and looking for "GNU".

My proposal is the following: when obtaining a succesful partial match (e.g. 
"kon" for "konsole", "kma" for "kmail" "sys" for "system settings") we store 
somewhere (in the querymatch?) not just the exact query (i.e. "kon", "kma", 
"sys") but the minimum completion of our partial query in the string (i.e. 
"konsole", "kmail", "system"). We will then keep track of the number of times a 
pair (match_id, "completed query") has been called instead of just the match_id.
Notice that we actually need the word(s) containing the query for this to be 
useful, otherwise "kon" "kons" "konso", willl all add up to different counters, 
which is suboptimal.
In any case this would imho allow krunner to better guess that the user wants 
to run for a given query.

* Better sorting II: at the moment the boost in relevance is linear in the 
number of runs (saturated to 1.0), therefore even partial matches can overcome 
exact matches if their corresponding entries have a very high run count. 

My proposal is to fix some given ranges for each match type (e.g. partial 
match, exact match, possible match) and saturate to the maximum allowed for 
this range instead that to 1.0; As a bonus the boost could be made quite easily 
non-linear in such a way that the saturation is smooth and not abrupt.

* Safer i18n support: a fix which got in just a little bit before release has 
to do with i18n. The issue here is that krunner is supposed to understand 
commands given in the user language. several runners try to obtain command 
parameters parsing the queries (e.g. the powerdevil runner accepts "power 
governor /performance/" and /performance/ is supposed to be a parameter; the 
solid runner accepts "mount /device/" and /device/ is a parameter). Most 
runners accomplish this task by splitting the query in a stringlist and then 
grabbing the relevant word(s). Of course this works in en_US, but possibly not 
in other languages, since the number of strings may vary, or the spaces between 
words may not be given by %20, who knows...

My proposal (thanks to a suggestion by Albert) is to make sure that each runner 
uses proper regular expressions for parameter extracting (afaict, the only 
runner featuring this in a (hopefully) correct way is the powerdevil runner).

As always, other ideas, comments or thoughts are more than welcome!
Regards

__J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: n+1st report about arrow keys in krunner

[Jacopo De Simoi]

OK, this morning I tried some quick hacks to see how this looks in practice and 
I have a few comments already:

1) binding focus of the first item with focus of the Combobox feels really 
natural so I'd commit to trunk as soon as I clean up the code.

2) using the down arrow to go down the result list directly from the combobox 
is also quite ok, it works and feels very natural for the single runner mode 
(since you have no history there). However there are a few issues which I 
encountered playing around with it in "regular" mode
  * as far as I can tell KHistoryCombobox gives no clue about wheter text has 
been entered by the user or has been selected from the history. This makes 
detecting "pressing Down without any further entry in the history" quite 
complicated in practice (unless we are willing to subclass the historycombobox 
or change the libs).
  * suppose there is a bunch of results and the user presses down several 
times; then the user wants to come back to the combobox and presses up 
several+1 times by accident.. suddenly the previous element in history is 
selected, results are changed, the user is now very confused.

A solution would be to act "symmetrically", i.e. to go back in history if and 
only if the down key has *not* been used to scroll down the list.
Con: adding further conditions may lead to rather non-intuitive behavior for a 
user which needs both features...

Another solution would be to change the shortcuts for the history handling 
(e.g. add a modifier)
Con: krunner should be thought as a minicli, and up/down in a minicli are 
supposed to handle history

Let me know your opinions 

__J

P.S.
Sorry if the code is not yet ready for trunk, I had only time to try some 
chainsaw patching here and there; in any case I feel it's constructive to post 
my impressions here and hope for some smart feedback. Of course I can attach a 
patch if you're willing to try it out on your local copy.

> Am Samstag 27 Februar 2010 23:09:57 schrieb Jacopo De Simoi:
> > If (and only if) pressing Down without any further entry in the history,
> > then it could move the focus to the first entry in the list.
> I would change to: If (and only if) pressing Down without any further entry 
> in 
> the history AND the Up key has not been used in this selection session.
> 
> If a user just presses down it can be expected he wants to use the list. If 
> he 
> used the up key before pressing the down key, he wants to browse the history. 
> So if he reaches the end of the history list again, we can expect that he 
> still wants to browse the history and not the list.
> 
> (Oh and thanks for the hint with the history browsing. Did not know about it 
> - 
> very useful feature ;-)
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: n+1st report about arrow keys in krunner

[Jacopo De Simoi]

> I have been annoyed many times by this, although I never reported a bug. I
> also found it confusing to have autocompletion, but I still have to type it
> through.

you can use the End key to accept the completion; this is another active topic 
about krunner on bko, by the way.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

n+1st report about arrow keys in krunner

[Jacopo De Simoi]

Dear plasmoids
  I just closed as WONTFIX the (n+1)st report about people trying to use 
downarrow key to select entries other than the first one in krunner default 
interface. In view of our committment to polish for 4.5 I guess it's time to do 
something about this issue, which seems to hit quite a number of users. 

The problem:
Krunner sports a command history feature similar to the konsole history, 
available by the uparrow and downarrow keys. 
The default interface shows results as a vertical list; this list can be 
browsed with the arrow keys once it has been focused (by pressing the TAB key). 
This focusing step is the one most users miss.
The confusion is probably made stronger by the fact that both the lineedit and 
the first entry are (virtually) focused at the same time, so one can reasonably 
expect the arrow keys to act on the result list instead that on the lineedit

Finding a consistent solution is not that simple, therefore I'd like to collect 
some proposals here, in order to have some time for testing during the .5 
development cycle. 

I report here a proposal by kdepepo (from the latest bugreport) along with my 
comment about it:

If (and only if) pressing Down without any further entry in the history, then
it could move the focus to the first entry in the list.

The first entry is already focused, we should probably rather move to the 
second one;
Once the second one has been focused,  what happens if i hit Up? should I 
return focus to the lineedit?
This would basically bind focus on the first item with focus of the lineedit. 
Would this actually work?

Awaiting for your comments and further proposals!
Thanks a lot

  __J



___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: [Kde-hardware-devel] Status of encrypted devices in Plasma / Solid

[Jacopo De Simoi]

> >  looking forward to check the validity of a bug regarding the device
> > notifier and encrypted devices (containers) I noticed several issues with
> > them in the current implementation of the dataengines
> > The imporant thing to know about them is that they show up in solid as a
> > StorageVolume such that StorageVolume.usage is 'Encrypted'. When they are
> > "mounted" solid asks for a password and if the password is correct, then
> > the partitions which are stored inside the device pop up and are correctly
> > collected by the hotplug dataengine. Then the user can use them normally;
> > the encrypted device (container) is closed (i.e. cannot be used anymore
> > without providing the password again) by "unmounting" it.
> 
> Indeed, pretty good summary of the behavior.
>  
> > A few issues arise in this situation:
> > 1) Solid cannot provide us with a signal when an encrypted container is
> > correctly "mounted" or "opened"
> 
> I'm not sure what makes encrypted containers special in this regard... The 
> setupDone() signal is emitted just like for any device supporting the 
> StorageAccess interface.

Correct, however if I remember correctly there is no signal such as 
"accessibilityChanged" which is catched by the dataengine to trigger a status 
change, since the object referred by the engine is not the one that requested 
the setup. 

> Now if you're talking about the missing {setup,teardown}RequestStarted signal 
> which would work accross process boundaries and that we discussed in 
> september, yes that's the same issue. But again, just to clarify, it has 
> nothing specific to the encrypted case.

No, indeed; moreover these signals are indeed quite useless if they are not 
emitted for all object referring to the same device. We would need to have 
those for all such objects to make good use of them. I'll post a new thread 
about this eventually.

> 
> > 2) The encrypted containers are currently
> > shown by the dataengine since they match the "open with file manager"
> > predicate, however this action does really nothing, since a container
> > simply contains partitions, so we can't really open the file manager on
> > unmounted partitions. Moreover, there is no real static action we can
> > associate with them. 3) The user is (imHo) misled by strings such as
> > "mount/unmount" and icons such as "mounted/unmounted/eject"
> > 
> > My proposed solutions:
> > 1) A solution (bad, but that's all we can do) to solve issue #1 is to let
> > the hotplug engine check if a newly added or removed device is inside an
> > encrypted container and trigger an update of the status of the container
> > accordingly.  If new devices are added it means that the container has
> > been opened if they are removed, it means that the container has been
> > closed.
> 
> Indeed nasty, I'd prefer to see it fixed below in the stack.

This is the workaround implemented in 4.4 for the dataengine

> 
> > 2) Rather than cheating giving a completely fake action we could
> > implement an exception for the hotplug engine, letting it populate devices
> > with encrypted devices even if they have no action associated with them.
> 
> It's definitely my favorite one.

Currently implemented in 4.4.

> 
> > This of course need adjustments to the two (that I know) users of the
> > dataengine (device notifier and solid runner) to make sure they don't
> > assume the existence of at least one action and in order for them to
> > properly handle such situation.
> 
> Well, it's really about making sure they don't make a (now) wrong assumption. 
> As a bonus that would shield them against a misbehaving engine, so that's 
> probably not a bad thing.
> 
> > 3) Nuno already kindly provided emblems/icons for the open/close status
> > (locked/unlocked); as for the strings I already asked for an exception for
> > string freeze regarding solid::Device::description() which should say
> > "encrypted container" instead of "removable media". I might as well ask
> > for an exception to add "Lock the container" and "unlock the container" if
> > felt needed. If string freeze exceptions for these are not granted I'd
> > rather leave descriptions empty rather than providing false information..
> 
> Since I suck and took months to get back to you I guess the string freeze is 
> not a problem anymore. :-)

The exception has been granted and the strings appear now in 4.4
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

services runner matching categories

[Jacopo De Simoi]

Dear plasma-devs

   After finally upgrading to 4.4 I noticed that the service runner started 
matching app categories; (the commit indeed happened ~6 months ago)
 which can be very handy for specific (i.e. single runner, plasma-netbook) 
queries, but might be imho a bit confusing in krunner common usage; the main 
problem is that such matches are marked as ExactMatches, which brings them on 
top of the results.

An example:
typing "system" shows (in this order in my testcase)
Konsole: terminal
Dolphin: file manager
Yakuake:drop-down terminal
kdiskfree: show disk usage

wireshark: network analyzer
system settings
system notifications
system bell
system monitor
sweeper: system cleaner

That is, the first term containing the word "system" is in 12th position; all 
the previous ones do not show the word "system" at all.
To me this is rather confusing as imho even a partial match on an application 
name should match "better" than an exact match on the relative category.

I can see how to get around this in a couple of ways:
1) demote exact matches to partial matches to make application names "win"; 
this is not perfect, since some popular applications (such as konsole and 
dolphin) will quicly climb up the result order; 
2) (quite drastic, indeed), match categories only in single runner mode, 
although I don't know if this feature has been added for the specific case of 
populating menus in various parts of the workspace or was meant to be something 
more general. In this case it probably should have been added with an 
appropriate keywording (e.g. category=).  I know it's too late to 
notice this. :(

Thoughts / comments?
Thanks
  --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: update the data when the predicate files change

[Jacopo De Simoi]

Ping 
  Any news on this? This  is meant to fix one of the last remaining bugs for 
the device notifier and it would be great to 
have it reviewed and merged soon
Thanks

On Monday 7 December 2009 18:38:59 Giulio Camuffo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2331/
> ---
> 
> (Updated 2009-12-07 17:38:58.991153)
> 
> 
> Review request for Plasma and Aaron Seigo.
> 
> 
> Changes
> ---
> 
> when the predicates are created check if there are some devices that now have 
> an action, while before they didn't; opposite thing on remove.
> 
> 
> Summary
> ---
> 
> This patch uses a KDirWatch to watch the folders where the predicate files 
> are in. If one of them gets created, modified, deleted, it updates its data 
> so the applets using it receive a dataUpdated() and can adjust their things.
> 
> 
> Diffs (updated)
> -
> 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/hotplug/CMakeLists.txt
>  1059391 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/hotplug/hotplugengine.h
>  1059391 
>   
> /trunk/KDE/kdebase/workspace/plasma/generic/dataengines/hotplug/hotplugengine.cpp
>  1059391 
> 
> Diff: http://reviewboard.kde.org/r/2331/diff
> 
> 
> Testing (updated)
> ---
> 
> tested, works. the notifier changes its "x actions for this device" 
> instantly. (the notifier needs a fix for it anyway, since the actions 
> themselves aren't changed. i'll do it once this is committed)
> 
> However, maybe would be a goog idea to cache somehow the value of the 
> function predicatesForDevice() in order to avoid to parse everytime all the 
> predicates and so parse only the new or changed ones, cause it seems to have 
> quite a big impact on performances.
> 
> 
> Thanks,
> 
> Giulio
> 
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Solid - places runners conflict

[Jacopo De Simoi]

On Wednesday 23 December 2009 06:50:50 Aaron J. Seigo wrote:
> On December 22, 2009, Jacopo De Simoi wrote:
> > I don't intend to disable the places runner itself, but just the matches
> >  referring to devices.
> 
> that's probably ok; my only concern would be where the user changes the name 
> of a label of a device in the places model. then it wouldn't match in 
> krunner, 

I am not able to change the label of a device with dophin, but I'm using 
dolphin and kfile libs from 4.3,
although I don't see any relevant change in websvn.
Can anybody confirm this?
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Solid - places runners conflict

[Jacopo De Simoi]

On Tuesday 22 December 2009 22:41:01 Aaron J. Seigo wrote:
> On December 18, 2009, Jacopo De Simoi wrote:
> > Dear plasmadevs,
> >   I just noticed that we have some feature conflict among the runners; in
> >  fact the places runner shows solid devices as well and triggers dolphin on
> >  them upon selection. Since this feature is included in solid runner, which
> >  is specialized to treat solid devices in a fairly complete way I propose
> >  to disable the feature from the places runner.
> 
> does the solid runner match custom items added to the places model?
> 
Nope, just devices. 

I don't intend to disable the places runner itself, but just the matches 
referring to devices.

--J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Solid - places runners conflict

[Jacopo De Simoi]

Dear plasmadevs,
  I just noticed that we have some feature conflict among the runners; in fact 
the places runner shows solid devices as well and triggers 
dolphin on them upon selection. Since this feature is included in solid runner, 
which is specialized to treat solid devices in a fairly complete way
I propose to disable the feature from the places runner.

Please let me know if there are objections / alternative solutions. 
Thanks a lot 

  --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Status of encrypted devices in Plasma / Solid

[Jacopo De Simoi]

Dear plasma-devs, hardware-devs
 looking forward to check the validity of a bug regarding the device notifier 
and encrypted devices (containers) I noticed several issues with them in the 
current implementation of 
the dataengines 
The imporant thing to know about them is that they show up in solid as a 
StorageVolume such that StorageVolume.usage is 'Encrypted'. When they are 
"mounted" 
solid asks for a password and if the password is correct, then the partitions 
which are stored inside the device pop up and are correctly collected by the 
hotplug dataengine.
Then the user can use them normally; the encrypted device (container) is closed 
(i.e. cannot be used anymore without providing the password again) by 
"unmounting" it.

A few issues arise in this situation:
1) Solid cannot provide us with a signal when an encrypted container is 
correctly "mounted" or "opened"
2) The encrypted containers are currently shown by the dataengine since they 
match the "open with file manager" predicate, however
this action does really nothing, since a container simply contains 
partitions, so we can't really open the file manager on unmounted partitions.
Moreover, there is no real static action we can associate with them.
3) The user is (imHo) misled by strings such as "mount/unmount" and icons such 
as "mounted/unmounted/eject"

My proposed solutions:
1) A solution (bad, but that's all we can do) to solve issue #1 is to let the 
hotplug engine check if a newly added or removed device is inside an
encrypted container and trigger an update of the status of the container 
accordingly.  If new devices are added it means that the container has been 
opened
if they are removed, it means that the container has been closed. 
2) Rather than cheating giving a completely fake action we could implement an 
exception for the hotplug engine, letting it populate devices with encrypted 
devices
even if they have no action associated with them.  This of course need 
adjustments to the two (that I know) users of the dataengine (device notifier 
and solid runner)
to make sure they don't assume the existence of at least one action and in 
order for them to properly handle such situation. 
A less pervasive solution would be to let the engine add dynamic action 
(open/close the container) if it detects an encrypted device. In this case the 
adjustments to the 
notifier and to the runner would be less pervasive, but still we need to 
treat the special action in a different way than the others, unless, we rely on 
soliduiserver::showActionsDialog
itself accepting these two as special actions.
3) Nuno already kindly provided emblems/icons for the open/close status 
(locked/unlocked); as for the strings I already asked for an exception for 
string freeze regarding 
solid::Device::description() which should say "encrypted container" instead 
of "removable media". I might as well ask for an exception to 
 add "Lock the container" and "unlock the container" if felt needed. If 
string freeze exceptions for these are not granted I'd rather leave 
descriptions empty rather than 
 providing false information..

Awaiting for your comments and suggestions 
Thanks

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

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

(Updated 2009-11-24 12:59:38.449340)


Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Changes
---

Do not load single runners if not selected; for now export them only if so, 
this will be dropped when loading-on-the-fly is implemented
this still needs to load them on-the-fly, but it can be easily fixed after 
commit


Summary
---

This is the API needed by the new single Runner mode for krunner (more on 
usecases in part II:krunner) for kdelibs;

A runner can now optionally define a Default syntax; if this is the case, the 
runner will be exposed as single-runner-mode capable. 
On request, the default syntax will be replaced to the empty query in 
RunnerManager::launchQuery.

The context is aware of the fact that we are in single runner query mode in 
case the runners need to change their behaviour accordingly (e.g. to _not_ 
discard queries with less than 3 chars)


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/abstractrunner.h 1052445 
  trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1052445 
  trunk/KDE/kdelibs/plasma/private/abstractrunner_p.h 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1053618 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1053618 
  trunk/KDE/kdelibs/plasma/runnermanager.h 1052445 
  trunk/KDE/kdelibs/plasma/runnermanager.cpp 1052445 

Diff: http://reviewboard.kde.org/r/2090/diff


Testing
---


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Solidrunner moved to kdereview

[Jacopo De Simoi]

Should I move it to trunk? 
If so, kdebase or kde-plasma-addons?
Thanks
  --J

On Friday 13 November 2009 19:33:57 Jacopo De Simoi wrote:
> I've just moved a the solidrunner to  kdereview, for inclusion in 4.4 in 
> kde-plasma-addons or kdebase; I'm actually not quite sure on that.
> 
> The runner is conceived as an alternative to the device notifier, combining 
> the same 
> capabilities (although the actions support is still lacking for the default 
> interface in krunner,
> I've already a working draft in local which will soon be posted to the 
> reviewboard) with
> the usual keyboard-oriented way of dealing with stuff which is peculiar to 
> krunner.
> 
> The runner features three syntaxes: 
> 
> device: show all devices 
> mount: show all devices that can be mounted
> unmount: show alll devices that can be unmounted
> 
> Since i believe that people using krunner are more CLI-oriented than the 
> average, the default action is mount/unmount/eject; actions provided by solid 
> actions will are exposed as actions;
> 
> It is also a perfect candidate for the single runner query mode (right now 
> the relevant part has been commented to allow compilation);
> 
> Please let me know of any issues that you find
> Thanks a lot
> 
>   --J
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add removeMatch(es) methods to runnerContext

[Jacopo De Simoi]

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

(Updated 2009-11-22 15:32:19.727345)


Review request for Plasma and Aaron Seigo.


Changes
---

whoops, unlock before bailing out!


Summary
---

This patch adds removeMatch and removeMatches methods to RunnerContext; these 
methods are needed to update results in krunner in case the runner knows that 
something changed and the previously reported results are not valid anymore 
(e.g. a window listed by the windows runner is closed [this currently leads to 
a crash] or a device is removed / changed its state (solid runner), a file has 
been deleted (places runner, nepomuk))

Both methods fire matchesChanged; in case the match is to be modified the 
runner can exploit the event compression done by runnermanager by removing the 
match and adding it back right away with the required modifications. 


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/querymatch.h 1052445 
  trunk/KDE/kdelibs/plasma/querymatch.cpp 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1052445 

Diff: http://reviewboard.kde.org/r/2260/diff


Testing
---

tested (in PoC stage) with the solidrunner and it works


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Plasma desktop crashed (mp3 player connected during startup)

[Jacopo De Simoi]

On Saturday 21 November 2009 20:44:32 l...@tlen.pl wrote:
> Device notifier automount version 0.3 crashes either when mp3 player is 
> connected during KDE startup
> or when I tried to remove mounted mp3 player 

devicenotifier_automount is an unofficial add-on and as such it is not 
supported by kde maintainers;
Please report the bug to the author, not to b.k.o.
Thanks

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add removeMatch(es) methods to runnerContext

[Jacopo De Simoi]

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

Review request for Plasma and Aaron Seigo.


Summary
---

This patch adds removeMatch and removeMatches methods to RunnerContext; these 
methods are needed to update results in krunner in case the runner knows that 
something changed and the previously reported results are not valid anymore 
(e.g. a window listed by the windows runner is closed [this currently leads to 
a crash] or a device is removed / changed its state (solid runner), a file has 
been deleted (places runner, nepomuk))

Both methods fire matchesChanged; in case the match is to be modified the 
runner can exploit the event compression done by runnermanager by removing the 
match and adding it back right away with the required modifications. 


Diffs
-

  trunk/KDE/kdelibs/plasma/querymatch.h 1052445 
  trunk/KDE/kdelibs/plasma/querymatch.cpp 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1052445 

Diff: http://reviewboard.kde.org/r/2260/diff


Testing
---

tested (in PoC stage) with the solidrunner and it works


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

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

(Updated 2009-11-21 18:49:12.791648)


Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Changes
---

forgot to add the relevant changes to the private header
Please let me know if the logic is ok (loading selected + singlequerymode at 
startup) or
if I should load SQM runners on the fly despite possibly long initialization 
times (yes I know we're working on that)


Summary
---

This is the API needed by the new single Runner mode for krunner (more on 
usecases in part II:krunner) for kdelibs;

A runner can now optionally define a Default syntax; if this is the case, the 
runner will be exposed as single-runner-mode capable. 
On request, the default syntax will be replaced to the empty query in 
RunnerManager::launchQuery.

The context is aware of the fact that we are in single runner query mode in 
case the runners need to change their behaviour accordingly (e.g. to _not_ 
discard queries with less than 3 chars)


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/abstractrunner.h 1052445 
  trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1052445 
  trunk/KDE/kdelibs/plasma/private/abstractrunner_p.h 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1052445 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1052445 
  trunk/KDE/kdelibs/plasma/runnermanager.h 1052445 
  trunk/KDE/kdelibs/plasma/runnermanager.cpp 1052445 

Diff: http://reviewboard.kde.org/r/2090/diff


Testing
---


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add action support to krunner default interface

[Jacopo De Simoi]

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

Review request for Plasma and Aaron Seigo.


Summary
---

This patch adds action support to the default interface by adding a button for 
each one of actionsForMatch given by the runnerManager. 
Hovering (and focusing) over a button changes the subtext of the item according 
to the selected action. 

The logic is all there, it has still some rough edges from the graphical side 
(e.g. we should make sure that there are not "too many" displayed actions) and 
focusing with the keyboard is missing (it should be done togheter with the 
configuration button) but these can be fixed quite easily once it gets 
committed.


Diffs
-

  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1052428 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1052428 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultitem.h 1052428 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultitem.cpp 1052428 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.h 1052428 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.cpp 
1052428 

Diff: http://reviewboard.kde.org/r/2259/diff


Testing
---

the only runner supporting actions I am aware of is the Devices (solid) runner 
(in kdereview); tested with that and it works.


Screenshots
---

Actions for the device runner
  http://reviewboard.kde.org/r/2259/s/269/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

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

(Updated 2009-11-20 07:55:33.801723)


Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Changes
---

updated diff to recent changes in trunk


Summary
---

This is the API needed by the new single Runner mode for krunner (more on 
usecases in part II:krunner) for kdelibs;

A runner can now optionally define a Default syntax; if this is the case, the 
runner will be exposed as single-runner-mode capable. 
On request, the default syntax will be replaced to the empty query in 
RunnerManager::launchQuery.

The context is aware of the fact that we are in single runner query mode in 
case the runners need to change their behaviour accordingly (e.g. to _not_ 
discard queries with less than 3 chars)


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/abstractrunner.h 1050953 
  trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1050953 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1050953 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1050953 
  trunk/KDE/kdelibs/plasma/runnermanager.h 1050953 
  trunk/KDE/kdelibs/plasma/runnermanager.cpp 1050953 

Diff: http://reviewboard.kde.org/r/2090/diff


Testing
---


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Update ItemBackground widget

[Jacopo De Simoi]

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

Ship it!


I should have removed that HACK at least one month ago. ;)
Thanks a lot!

- Jacopo


On 2009-11-17 20:31:35, Artur de Souza (MoRpHeUz) wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2194/
> ---
> 
> (Updated 2009-11-17 20:31:35)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> Remove code that was depending on Qt 4.6 (kde already depends on that).
> Also try to fix a bug on device notifier crashing on this hide() at 
> setItem(0).
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdelibs/plasma/widgets/itembackground.cpp 1050478 
> 
> Diff: http://reviewboard.kde.org/r/2194/diff
> 
> 
> Testing
> ---
> 
> Tested with trunk and qt 4.6-rc1. Seems to be working but other people 
> testing this would be awesome.
> 
> 
> Thanks,
> 
> Artur
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Solidrunner moved to kdereview

[Jacopo De Simoi]

now the solidrunner is in the more appropriate location

kdereview/plasma/runners

thanks for reviewing!

On Friday 13 November 2009 19:33:57 Jacopo De Simoi wrote:
> I've just moved a the solidrunner to  kdereview, for inclusion in 4.4 in 
> kde-plasma-addons or kdebase; I'm actually not quite sure on that.
> 
> The runner is conceived as an alternative to the device notifier, combining 
> the same 
> capabilities (although the actions support is still lacking for the default 
> interface in krunner,
> I've already a working draft in local which will soon be posted to the 
> reviewboard) with
> the usual keyboard-oriented way of dealing with stuff which is peculiar to 
> krunner.
> 
> The runner features three syntaxes: 
> 
> device: show all devices 
> mount: show all devices that can be mounted
> unmount: show alll devices that can be unmounted
> 
> Since i believe that people using krunner are more CLI-oriented than the 
> average, the default action is mount/unmount/eject; actions provided by solid 
> actions will are exposed as actions;
> 
> It is also a perfect candidate for the single runner query mode (right now 
> the relevant part has been commented to allow compilation);
> 
> Please let me know of any issues that you find
> Thanks a lot
> 
>   --J
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

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

(Updated 2009-11-17 17:52:14.417634)


Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Changes
---

In order for the runner to be exposed by krunner as single runner query mode 
(SRQM) enabled, the relative .desktop file must now provide the 
"SingleRunnerQueryMode" key set to true; this allows the runnerManager to load 
and keep loaded the SRQM-enabled runners even if the user does not want them in 
the main krunner window results. 
A certain amount of bookkeeping has to be done to ensure that everything works 
properly; the implementation attempts to minimize that and minimize the risk of 
breakage.

All previous review comments have been taken into account


Summary
---

This is the API needed by the new single Runner mode for krunner (more on 
usecases in part II:krunner) for kdelibs;

A runner can now optionally define a Default syntax; if this is the case, the 
runner will be exposed as single-runner-mode capable. 
On request, the default syntax will be replaced to the empty query in 
RunnerManager::launchQuery.

The context is aware of the fact that we are in single runner query mode in 
case the runners need to change their behaviour accordingly (e.g. to _not_ 
discard queries with less than 3 chars)


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
  trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1048133 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1050508 
  trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
  trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 

Diff: http://reviewboard.kde.org/r/2090/diff


Testing
---


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/abstractrunner.h, line 406
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13849#file13849line406>
> >
> > this should probably be setDefaultSyntax, since it doesn't really add 
> > another default syntax. also prevents the need to explain how it works in 
> > the apidox :)
> 
> Jacopo De Simoi wrote:
> well, the correct name should indeed be
> addSyntaxAndSetAsDefault, but I guess it's a little bit too much :)
> 
> Aaron Seigo wrote:
> i think setDefaultSyntax implies it will become part of the syntax bunch; 
> perhaps a note in the apidox that it isn't necessary to to also call 
> addSyntax and a check to ensure it isn't already in the collection might be 
> in order.
> 
> Jacopo De Simoi wrote:
> agreed

Done, (+note in the apidox) 
However checking that the syntax is already in the collection is complicated by 
the fact that we need to define what the operator== means for a syntax;
besides, it should probably be added for addSyntax as well...


- Jacopo


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


On 2009-11-07 20:16:02, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2090/
> ---
> 
> (Updated 2009-11-07 20:16:02)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This is the API needed by the new single Runner mode for krunner (more on 
> usecases in part II:krunner) for kdelibs;
> 
> A runner can now optionally define a Default syntax; if this is the case, the 
> runner will be exposed as single-runner-mode capable. 
> On request, the default syntax will be replaced to the empty query in 
> RunnerManager::launchQuery.
> 
> The context is aware of the fact that we are in single runner query mode in 
> case the runners need to change their behaviour accordingly (e.g. to _not_ 
> discard queries with less than 3 chars)
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
>   trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 
> 
> Diff: http://reviewboard.kde.org/r/2090/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part II: krunner)

[Jacopo De Simoi]

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

(Updated 2009-11-16 23:50:00.387923)


Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Changes
---

Updated screenshot; waiting for the other part of the patch 
(http://reviewboard.kde.org/r/2090/) to be better defined to post the patch 
here against it.


Summary
---

This patch adds single runner query support to krunner;
When loading the runners, if any runner supports a defaultSyntax, then this is 
exposed via an action which shows up in the global shortcuts;
When this action is triggered, krunner starts in single-runner-query mode; i.e. 
queries are processed by the selected runner only; the interfaces changes 
slightly to reflect this fact  (screenshot) and the query given by the default 
syntax is automatically run.

Also a convenient dbus interface is added, in case we want to trigger this 
internally; 

The most obvious usecase is the windows runner; an upcoming devicerunner and 
also the nepomuk search runner could make use of this feature.


Diffs
-

  trunk/KDE/kdebase/workspace/krunner/dbus/org.kde.krunner.App.xml 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.cpp 
1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.cpp 
1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerdialog.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerdialog.cpp 1046190 

Diff: http://reviewboard.kde.org/r/2091/diff


Testing
---


Screenshots (updated)
---

Window runner in single runner mode (+descriptive label)
  http://reviewboard.kde.org/r/2091/s/264/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part II: krunner)

[Jacopo De Simoi]

> On 2009-11-12 20:53:01, Aaron Seigo wrote:
> > the line edit for the single query mode should probably have a click text 
> > that comes from the name or description of the runner being used; maybe it 
> > would even be useful to put a label next to the icon with the name of the 
> > runner, so it's really evident what's going on?
> 
> Jacopo De Simoi wrote:
> Clickmessage would be cool, however, since the lineedit immediately gets 
> focus, it is never shown so it is useless.
> A label could be a solution indeed; 
> The issue is: what should the label read? just the name of the runner?
> 
> Aaron Seigo wrote:
> the name of the runner should be enough; hopefully it also looks good :)

The name of the runner looks good enough; I'll go with it


- Jacopo


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


On 2009-11-16 23:50:00, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2091/
> ---
> 
> (Updated 2009-11-16 23:50:00)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This patch adds single runner query support to krunner;
> When loading the runners, if any runner supports a defaultSyntax, then this 
> is exposed via an action which shows up in the global shortcuts;
> When this action is triggered, krunner starts in single-runner-query mode; 
> i.e. queries are processed by the selected runner only; the interfaces 
> changes slightly to reflect this fact  (screenshot) and the query given by 
> the default syntax is automatically run.
> 
> Also a convenient dbus interface is added, in case we want to trigger this 
> internally; 
> 
> The most obvious usecase is the windows runner; an upcoming devicerunner and 
> also the nepomuk search runner could make use of this feature.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdebase/workspace/krunner/dbus/org.kde.krunner.App.xml 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.h 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerapp.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerdialog.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerdialog.cpp 1046190 
> 
> Diff: http://reviewboard.kde.org/r/2091/diff
> 
> 
> Testing
> ---
> 
> 
> Screenshots
> ---
> 
> Window runner in single runner mode (+descriptive label)
>   http://reviewboard.kde.org/r/2091/s/264/
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/runnermanager.h, line 149
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13853#file13853line149>
> >
> > const bool isn't necessary; in fact, i think this whole method 
> > shouldn't be necessary at all. if RunnerContext::singleRunnerMode() then 
> > replaceEmptyQuery should be implied?
> 
> Jacopo De Simoi wrote:
> In fact it should. I was worried of changing the default behaviour of 
> this method. I was not sure to be able to check that this wouldn't affect any 
> other app using runnermanager (kickoff?)
> 
> Aaron Seigo wrote:
> i don't see how it could affect any other users; if they don't call 
> setSingleRunnerMode(true) it behaves exactly as it did before. so it would 
> require purposefully changing the code in the other places it is used to 
> cause problems :)
> 
> Jacopo De Simoi wrote:
> aw, I should really read the whole comment before replying. In fact your 
> comment makes perfect sense

I am trying to fix this and now I remember what didn't work without that bool.
The point is that launchQuery resets() the context, hence it is launchQuery 
itself that sets the single runner query mode; thus it makes little sense to 
check RunnerContext::singleRunnerMode(); and I am not sure if assuming that the 
empty query should always be substituted if a runner is specified would break 
other apps.
What should I do then? keep the bool? any alternative solutions?


- Jacopo


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


On 2009-11-07 20:16:02, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2090/
> ---
> 
> (Updated 2009-11-07 20:16:02)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This is the API needed by the new single Runner mode for krunner (more on 
> usecases in part II:krunner) for kdelibs;
> 
> A runner can now optionally define a Default syntax; if this is the case, the 
> runner will be exposed as single-runner-mode capable. 
> On request, the default syntax will be replaced to the empty query in 
> RunnerManager::launchQuery.
> 
> The context is aware of the fact that we are in single runner query mode in 
> case the runners need to change their behaviour accordingly (e.g. to _not_ 
> discard queries with less than 3 chars)
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
>   trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 
> 
> Diff: http://reviewboard.kde.org/r/2090/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Solidrunner moved to kdereview

[Jacopo De Simoi]

On Friday 13 November 2009 20:06:48 Burkhard Lück wrote:
> Am Freitag, 13. novembre 2009 19:33:57 schrieb Jacopo De Simoi:
> > 
> > Please let me know of any issues that you find
> 
> There are some i18n calls, but the runner has no translation catalog, message 
> extraction via Messages.sh is missing.

Fixed, thanks
  --J

> 
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Solidrunner moved to kdereview

[Jacopo De Simoi]

I've just moved a the solidrunner to  kdereview, for inclusion in 4.4 in 
kde-plasma-addons or kdebase; I'm actually not quite sure on that.

The runner is conceived as an alternative to the device notifier, combining the 
same 
capabilities (although the actions support is still lacking for the default 
interface in krunner,
I've already a working draft in local which will soon be posted to the 
reviewboard) with
the usual keyboard-oriented way of dealing with stuff which is peculiar to 
krunner.

The runner features three syntaxes: 

device: show all devices 
mount: show all devices that can be mounted
unmount: show alll devices that can be unmounted

Since i believe that people using krunner are more CLI-oriented than the 
average, the default action is mount/unmount/eject; actions provided by solid 
actions will are exposed as actions;

It is also a perfect candidate for the single runner query mode (right now the 
relevant part has been commented to allow compilation);

Please let me know of any issues that you find
Thanks a lot

  --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/runnermanager.h, line 149
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13853#file13853line149>
> >
> > const bool isn't necessary; in fact, i think this whole method 
> > shouldn't be necessary at all. if RunnerContext::singleRunnerMode() then 
> > replaceEmptyQuery should be implied?
> 
> Jacopo De Simoi wrote:
> In fact it should. I was worried of changing the default behaviour of 
> this method. I was not sure to be able to check that this wouldn't affect any 
> other app using runnermanager (kickoff?)
> 
> Aaron Seigo wrote:
> i don't see how it could affect any other users; if they don't call 
> setSingleRunnerMode(true) it behaves exactly as it did before. so it would 
> require purposefully changing the code in the other places it is used to 
> cause problems :)

aw, I should really read the whole comment before replying. In fact your 
comment makes perfect sense 


> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/abstractrunner.h, line 406
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13849#file13849line406>
> >
> > this should probably be setDefaultSyntax, since it doesn't really add 
> > another default syntax. also prevents the need to explain how it works in 
> > the apidox :)
> 
> Jacopo De Simoi wrote:
> well, the correct name should indeed be
> addSyntaxAndSetAsDefault, but I guess it's a little bit too much :)
> 
> Aaron Seigo wrote:
> i think setDefaultSyntax implies it will become part of the syntax bunch; 
> perhaps a note in the apidox that it isn't necessary to to also call 
> addSyntax and a check to ensure it isn't already in the collection might be 
> in order.

agreed


- Jacopo


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


On 2009-11-07 20:16:02, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2090/
> ---
> 
> (Updated 2009-11-07 20:16:02)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This is the API needed by the new single Runner mode for krunner (more on 
> usecases in part II:krunner) for kdelibs;
> 
> A runner can now optionally define a Default syntax; if this is the case, the 
> runner will be exposed as single-runner-mode capable. 
> On request, the default syntax will be replaced to the empty query in 
> RunnerManager::launchQuery.
> 
> The context is aware of the fact that we are in single runner query mode in 
> case the runners need to change their behaviour accordingly (e.g. to _not_ 
> discard queries with less than 3 chars)
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
>   trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 
> 
> Diff: http://reviewboard.kde.org/r/2090/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part II: krunner)

[Jacopo De Simoi]

> On 2009-11-12 20:53:01, Aaron Seigo wrote:
> > the line edit for the single query mode should probably have a click text 
> > that comes from the name or description of the runner being used; maybe it 
> > would even be useful to put a label next to the icon with the name of the 
> > runner, so it's really evident what's going on?

Clickmessage would be cool, however, since the lineedit immediately gets focus, 
it is never shown so it is useless.
A label could be a solution indeed; 
The issue is: what should the label read? just the name of the runner?


> On 2009-11-12 20:53:01, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp, lines 
> > 446-449
> > <http://reviewboard.kde.org/r/2091/diff/1/?file=13857#file13857line446>
> >
> > adjustInterface(bool) is redundant; may as well just have one method, 
> > setStaticQueryMode(bool)

my only concern was to give methods a meaningful name; adjustInterface does 
more than setting the static query mode or not


> On 2009-11-12 20:53:01, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp, line 305
> > <http://reviewboard.kde.org/r/2091/diff/1/?file=13862#file13862line305>
> >
> > isn't that a no-op due to:
> > 
> > void KRunnerApp::singleRunnerMode(const QString& runnerName)
> > {
> > if ( runnerName.isEmpty() ) {
> > return;

actually it's 

void KRunnerDialog::setSingleRunnerMode(QString runnerName)
{
m_singleRunnerId = runnerName;

if (runnerName.isEmpty()) {
return;
}
...

KRunnerApp::singleRunnermode is the dbus method, whose name will be changed :)


- Jacopo


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


On 2009-11-07 20:29:26, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2091/
> ---
> 
> (Updated 2009-11-07 20:29:26)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This patch adds single runner query support to krunner;
> When loading the runners, if any runner supports a defaultSyntax, then this 
> is exposed via an action which shows up in the global shortcuts;
> When this action is triggered, krunner starts in single-runner-query mode; 
> i.e. queries are processed by the selected runner only; the interfaces 
> changes slightly to reflect this fact  (screenshot) and the query given by 
> the default syntax is automatically run.
> 
> Also a convenient dbus interface is added, in case we want to trigger this 
> internally; 
> 
> The most obvious usecase is the windows runner; an upcoming devicerunner and 
> also the nepomuk search runner could make use of this feature.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdebase/workspace/krunner/dbus/org.kde.krunner.App.xml 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.h 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.cpp 
> 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerapp.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerdialog.h 1046190 
>   trunk/KDE/kdebase/workspace/krunner/krunnerdialog.cpp 1046190 
> 
> Diff: http://reviewboard.kde.org/r/2091/diff
> 
> 
> Testing
> ---
> 
> 
> Screenshots
> ---
> 
> Window runner in single runner mode
>   http://reviewboard.kde.org/r/2091/s/255/
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/abstractrunner.h, line 406
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13849#file13849line406>
> >
> > this should probably be setDefaultSyntax, since it doesn't really add 
> > another default syntax. also prevents the need to explain how it works in 
> > the apidox :)

well, the correct name should indeed be
addSyntaxAndSetAsDefault, but I guess it's a little bit too much :)


> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/runnercontext.h, lines 180-181
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13851#file13851line180>
> >
> > sounds like something that should take a bool?
> > 
> > also, some extra ws snuck in there :)

agreed


> On 2009-11-12 20:39:42, Aaron Seigo wrote:
> > trunk/KDE/kdelibs/plasma/runnermanager.h, line 149
> > <http://reviewboard.kde.org/r/2090/diff/1/?file=13853#file13853line149>
> >
> > const bool isn't necessary; in fact, i think this whole method 
> > shouldn't be necessary at all. if RunnerContext::singleRunnerMode() then 
> > replaceEmptyQuery should be implied?

In fact it should. I was worried of changing the default behaviour of this 
method. I was not sure to be able to check that this wouldn't affect any other 
app using runnermanager (kickoff?)


- Jacopo


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


On 2009-11-07 20:16:02, Jacopo De Simoi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2090/
> ---
> 
> (Updated 2009-11-07 20:16:02)
> 
> 
> Review request for Plasma, Aaron Seigo and Ryan Bitanga.
> 
> 
> Summary
> ---
> 
> This is the API needed by the new single Runner mode for krunner (more on 
> usecases in part II:krunner) for kdelibs;
> 
> A runner can now optionally define a Default syntax; if this is the case, the 
> runner will be exposed as single-runner-mode capable. 
> On request, the default syntax will be replaced to the empty query in 
> RunnerManager::launchQuery.
> 
> The context is aware of the fact that we are in single runner query mode in 
> case the runners need to change their behaviour accordingly (e.g. to _not_ 
> discard queries with less than 3 chars)
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
>   trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnercontext.cpp 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
>   trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 
> 
> Diff: http://reviewboard.kde.org/r/2090/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacopo
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Adds windows runner single runner query support

[Jacopo De Simoi]

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

Review request for Plasma, Aaron Seigo and Martin Gräßlin.


Summary
---

This patch allows the windows runner to provide single runner support using 
"window" as the default query


Diffs
-

  trunk/KDE/kdebase/workspace/plasma/generic/runners/windows/windowsrunner.cpp 
1043023 

Diff: http://reviewboard.kde.org/r/2092/diff


Testing
---

Works well


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add support for single Runner queries to krunner (part II: krunner)

[Jacopo De Simoi]

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

Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Summary
---

This patch adds single runner query support to krunner;
When loading the runners, if any runner supports a defaultSyntax, then this is 
exposed via an action which shows up in the global shortcuts;
When this action is triggered, krunner starts in single-runner-query mode; i.e. 
queries are processed by the selected runner only; the interfaces changes 
slightly to reflect this fact  (screenshot) and the query given by the default 
syntax is automatically run.

Also a convenient dbus interface is added, in case we want to trigger this 
internally; 

The most obvious usecase is the windows runner; an upcoming devicerunner and 
also the nepomuk search runner could make use of this feature.


Diffs
-

  trunk/KDE/kdebase/workspace/krunner/dbus/org.kde.krunner.App.xml 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/resultscene.cpp 
1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/interfaces/quicksand/qs_dialog.cpp 
1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerdialog.h 1046190 
  trunk/KDE/kdebase/workspace/krunner/krunnerdialog.cpp 1046190 

Diff: http://reviewboard.kde.org/r/2091/diff


Testing
---


Screenshots
---

Window runner in single runner mode
  http://reviewboard.kde.org/r/2091/s/255/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add support for single Runner queries to krunner (part I: libplasma)

[Jacopo De Simoi]

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

Review request for Plasma, Aaron Seigo and Ryan Bitanga.


Summary
---

This is the API needed by the new single Runner mode for krunner (more on 
usecases in part II:krunner) for kdelibs;

A runner can now optionally define a Default syntax; if this is the case, the 
runner will be exposed as single-runner-mode capable. 
On request, the default syntax will be replaced to the empty query in 
RunnerManager::launchQuery.

The context is aware of the fact that we are in single runner query mode in 
case the runners need to change their behaviour accordingly (e.g. to _not_ 
discard queries with less than 3 chars)


Diffs
-

  trunk/KDE/kdelibs/plasma/abstractrunner.h 1045930 
  trunk/KDE/kdelibs/plasma/abstractrunner.cpp 1045930 
  trunk/KDE/kdelibs/plasma/runnercontext.h 1045930 
  trunk/KDE/kdelibs/plasma/runnercontext.cpp 1045930 
  trunk/KDE/kdelibs/plasma/runnermanager.h 1045930 
  trunk/KDE/kdelibs/plasma/runnermanager.cpp 1045930 

Diff: http://reviewboard.kde.org/r/2090/diff


Testing
---


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Embed related kcms in the device notifier configuration dialog

[Jacopo De Simoi]

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

(Updated 2009-10-31 10:40:50.365695)


Review request for Plasma, Aaron Seigo, Trever Fischer, Kevin Ottens, and Ben 
Cooksley.


Changes
---

Now the kcms are embedded in the notifier configuration dialog; slickness++


Summary (updated)
---

This patch adds to the configuration interface two buttons which links to two 
related kcms; 

(btw, what is the situation with the device-automounter? I could only find a 
copy in kdereview)

* If you think it's worth to add such options, please help with the naming.. 
I've no good ideas today ;) 
* Is there a template for non-trivial plasmoid config uis?   


This addresses bugs 187054 and 194894.
https://bugs.kde.org/show_bug.cgi?id=187054
https://bugs.kde.org/show_bug.cgi?id=194894


Diffs (updated)
-

  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/CMakeLists.txt
 1040042 
  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.cpp
 1040042 

Diff: http://reviewboard.kde.org/r/1979/diff


Testing
---

It works as expected


Screenshots (updated)
---

new config window [2]
  http://reviewboard.kde.org/r/1979/s/243/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add to the devicenotifier configuration buttons to open related kcms

[Jacopo De Simoi]

On Tuesday 27 October 2009 17:16:58 Aaron Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1979/#review2832
> ---
> 
> 
> instead of launching kcmshell, you should be able to just KRun::run the 
> service which can be retrieved by name. however, i wonder if it wouldn't make 
> more sense to show the configuration pages right inside the plasmoid config 
> dialog? this should be possible by using KCModuleProxy and then adding that 
> as a page to the dialog.

/me likes! I'll take care of that

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add to the devicenotifier configuration buttons to open related kcms

[Jacopo De Simoi]

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

(Updated 2009-10-26 18:33:26.585079)


Review request for Plasma, Aaron Seigo, Trever Fischer, and Kevin Ottens.


Changes
---

I should really learn to write more descriptive summaries ;)


Summary (updated)
---

This patch adds to the configuration interface two buttons which links to two 
related kcms; 

(btw, what is the situation with the device-automounter? I could only find a 
copy in kdereview)

* If you think it's worth to add such options, please help with the naming.. 
I've no good ideas today ;) 
* Is there a template for non-trivial plasmoid config uis?   


This addresses bugs 187054 and 194894.
https://bugs.kde.org/show_bug.cgi?id=187054
https://bugs.kde.org/show_bug.cgi?id=194894


Diffs
-

  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/configurationpage.ui
 1040042 
  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.h
 1040042 
  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.cpp
 1040042 

Diff: http://reviewboard.kde.org/r/1979/diff


Testing
---

It works as expected


Screenshots
---

new config window
  http://reviewboard.kde.org/r/1979/s/239/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add to the configuration interface buttons calling the kcms for device actions and automounting

[Jacopo De Simoi]

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

Review request for Plasma, Aaron Seigo, Trever Fischer, and Kevin Ottens.


Summary
---

This patch adds to the configuration interface two buttons which links to two 
related kcms; 

(btw, what is the situation with the device-automounter? I could only find a 
copy in kdereview)

* If you think it's worth to add such options, please help with the naming.. 
I've no good ideas today ;) 
* Is there a template for non-trivial plasmoid config uis?   


This addresses bugs 187054 and 194894.
https://bugs.kde.org/show_bug.cgi?id=187054
https://bugs.kde.org/show_bug.cgi?id=194894


Diffs
-

  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/configurationpage.ui
 1040042 
  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.h
 1040042 
  
trunk/KDE/kdebase/workspace/plasma/generic/applets/devicenotifier/devicenotifier.cpp
 1040042 

Diff: http://reviewboard.kde.org/r/1979/diff


Testing
---

It works as expected


Screenshots
---

new config window
  http://reviewboard.kde.org/r/1979/s/239/


Thanks,

Jacopo

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

On Tuesday 06 October 2009 22:17:02 Aaron J. Seigo wrote:
> so much for simplicity :/
 
I'm confused; I can read in test-predicate-openinwindow.desktop:

X-KDE-Solid-Predicate=[ [ StorageVolume.ignored == false AND [ 
StorageVolume.usage == 'FileSystem' OR StorageVolume.usage == 'Encrypted' ] ] 
OR [ IS StorageAccess AND StorageDrive.driveType == 'Floppy' ] ]

Removing the first check should be enough; why is it not so?

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

On Tuesday 06 October 2009 21:58:08 Aaron J. Seigo wrote:
> >  i think anyway a better approach would be to add an action
> >  file for them, and add a property in the DeviceItem to set if they are
> >  (un)mountable
> 
> it would prevent any special-case code in the notifier widget, yes.

If we have no actions for the device, what should we offer? just 
mount/unmounting? 
To me it would make sense to offer at least "open with %filemanager" for 
devices marked with the ignore flag.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Unified ItemBackground in new Device Notifier

[Jacopo De Simoi]

> On 2009-10-06 10:16:00, Jacopo De Simoi wrote:
> > /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp, 
> > line 390
> > <http://reviewboard.kde.org/r/1790/diff/3/?file=12370#file12370line390>
> >
> > Now that the left(hehe)action stays activated when !isCollapsed() we 
> > should call m_leftActionIcon->setVisible(m_hovered) here
> >
> 
>  wrote:
> the real problem here is that mouse release off of the item is accepted; 
> it shouldn't be possible to collapse with a mouse click and not still be 
> hovered. fixed in my local copy.

indeed it /is/ possible to collapse one item simply by expanding another item, 
without touching the former.


> On 2009-10-06 10:16:00, Jacopo De Simoi wrote:
> > /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp, 
> > line 484
> > <http://reviewboard.kde.org/r/1790/diff/3/?file=12372#file12372line484>
> >
> > We have a problem here: 
> > itemHovered is connected to targetReached, however since now we have a 
> > timer (100ms) to hide the itemBackground it happens that a user could 
> > hoverLeave the selected item right before this signal is triggered; which 
> > leaves the item in the hovered state.
> > 
> > I suggest adding a m_hoveredItem which is set/reset in eventFilter and 
> > is "promoted" to m_selectedDevice here.
> > itemHovered should be called only if (item == m_hoveredItem)
> >
> 
>  wrote:
> item->setHovered(false) is called on HoverLeave in 
> NotifierDialog::eventFilter. the timer is not responsible for setting an item 
> as not hovered, only for setting it as hovered.
> 
> as for the timer, m_clearItemBackgroundTargetTimer.start() is called when 
> any device item gets a HoverLeave, collapsed or selected. it is also called 
> when a selected item gets a HoverEnter. so it is always started except when a 
> non-collapsed (aka selected) item gets a HoverEnter. i don't see where this 
> timer could get out of step of the events.
> 
> so i don't see a problem in either case.

The problem is the following: suppose you have 2 items: x and y; all times are 
expressed in ms:

at time 0 you leave item x 
at time 50 you hover on item y, m_itemBackground starts its animation from item 
x to item y
at time 300 you leave item y and you do not hover any other item (e.g. out of 
the plasmoid), the timer is started; y->setHovered(false)
at time 350 m_itemBackground arrives on y, signals targetReached, which calls 
itemHovered, which calls y->setHovered(true)
at time 400 m_itemBackground gets hidden, but nobody resets the status of item 
y, which stays in the "hovered" state.

Despite the fact that is seems complicated to reproduce, just waving your mouse 
over the plasmoid triggers this situation


> On 2009-10-06 10:16:00, Jacopo De Simoi wrote:
> > /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp, 
> > line 152
> > <http://reviewboard.kde.org/r/1790/diff/3/?file=12370#file12370line152>
> >
> > Here we should make sure to hide() the description Label;
> 
>  wrote:
> playing around with it some more, it feels odd to have some parts 
> disappearing and others staying ... i've made all the header items remain 
> when the item is expanded. the downside to this is that the capacity bar 
> won't update when the item is expanded.

As for making the description label less redundant, if we really feel bad about 
it, we could add a ":" at the end when the device gets expanded...

However, the capacity bar  should /always/ display a proper value, I understand 
that it would get updated on hover anyways, but that seems to me more broken 
than hiding/showing it.

Perhaps we should show it in a less glamorous way, for instance as a tiny 
capacity bar below the icon (a few pixels high) and the label (xxx Gib free) 
below the description... then seeing it appear and disappear should bother the 
user that much...

Mmh, that doesn't even convince me.


- Jacopo


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


On 2009-10-05 23:27:10, Aaron Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1790/
> ---
> 
> (Updated 2009-10-05 23:27:10)
> 
> 
> Review request for Plasma, Jacopo De Simoi and Giulio Camuffo.
> 
> 
> Summary
> ---
> 
> proof of concept

Re: Review Request: Unified ItemBackground in new Device Notifier

[Jacopo De Simoi]

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

Ship it!


I'm finally convinced, we need to fix some issues here and there, but we can do 
it after the commit


/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1884>

Here we should now make sure to setVisible(m_hovered) the descriptionLabel



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1883>

Here we should make sure to hide() the description Label; 



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1878>

shouldn't this be (m_hovered || !isCollapsed()) as well?



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1881>

Now that the left(hehe)action stays activated when !isCollapsed() we should 
call m_leftActionIcon->setVisible(m_hovered) here




/trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp
<http://reviewboard.kde.org/r/1790/#comment1882>

We have a problem here: 
itemHovered is connected to targetReached, however since now we have a 
timer (100ms) to hide the itemBackground it happens that a user could 
hoverLeave the selected item right before this signal is triggered; which 
leaves the item in the hovered state.

I suggest adding a m_hoveredItem which is set/reset in eventFilter and is 
"promoted" to m_selectedDevice here.
itemHovered should be called only if (item == m_hoveredItem)
   


- Jacopo


On 2009-10-05 23:27:10, Aaron Seigo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1790/
> ---
> 
> (Updated 2009-10-05 23:27:10)
> 
> 
> Review request for Plasma, Jacopo De Simoi and Giulio Camuffo.
> 
> 
> Summary
> ---
> 
> proof of concept showing how the item background could be unified into 
> NotifierDialog; requires today's libplasma for fixes to the ItemBackground 
> widget
> 
> 
> Diffs
> -
> 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.h 
> 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp 
> 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.h 
> 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp 
> 1031762 
> 
> Diff: http://reviewboard.kde.org/r/1790/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aaron
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

On Sunday 04 October 2009 22:31:49 Aaron J. Seigo wrote:

> > > * when an item is expanded, should the background remain painted? it
> > > might make it more evident that the item is "open", and it would also
> > > allow a way to solve the next point, too. (and if the background remains,
> > > perhaps the capacity bar should too?)
> > 
> > The background could  in principle stay, for the capacity bar there is an
> >  issue: there is no way to make a refresh signal "free space changed";
> >  that's why we show it only on hover (and for the same reason it is like
> >  that for kfileplacesitem);
> 
> ah, ok.. unfortunate but sensible (and now i remember the conversation about 
> this a while back on kde-core-devel or kfm-devel as well :). perhaps the 
> action icon (Eject) could remain at least? 
Yes, I don't see any drawback in doing that

> ...
> consider two items, both with multiple actions and both of which have been 
> clicked to expand...
only one Device is allowed to be expanded at the same time; this was made to 
help the user to keep the notifier tidy and avoid it to grow too much in the 
vertical direction..
> .. moving from the second action in the first device to the 
> first action in the second device would feel quite "natural" and smooth if 
> the 
> ItemBackground moved between them.
So your idea would be to use the itemBackground in the notifierdialog for 
closed devices and actions of open devices, and to set another itemBackground 
for the item if it is opened. 

I'd have to see it in action to understand if I like it; 
the current behaviour intended to instantly communicate to the user that there 
is a hierarchical structure between categories, devices and actions, that is 
why each device has (visually) its own itemBackground for actions
 
> * the popup icon only changes when the user will be notified of a device. 
> would it make sense to show an icon change even if the device is going to be 
> ignored? that way there would at least be feedback that the applet was aware 
> there was a change and chose not to do anything about it?
(see later)

> * i really like how the unmounting waiting is shown; i did notice that the 
> icon overlay on a storage volume for mounted matches the action icon to mount 
> a device; but when you click on unmount, the overlay is a box with a line 
> through it. this seems asymmetrical. should the overlay on the storage volume 
> icon be the "unmount" icon?

the icon overlay is an "emblem", which is given to us directly by solid to 
reflect the state of the device and it is also used, for instance, by 
KFilePlacesItem and friends;  the "box with a line" is the "unmounted" emblem 
(thanks Nuno!) which, at least to me, fits quite well the idea of "not 
accessible". On the other hand the action on the right is, indeed, an action 
icon (not a state), and the "eject" button fits quite well the idea of 
"unmounting" (vs "unmounted"). 
 
> would it make sense to show some sort of feedback when a hidden item is 
> plugged in? the popup wouldn't need to show up and a full DeviceItem wouldn't 
> need to be constructed or anything, but perhaps a little "A hidden device was 
> plugged in" entry that could be clicked on to show the device if you wanted 
> that disappears automatically after N seconds?

What about this:
When at least one hidden device is plugged in, there will be a sort of "status 
label" at the bottom of the widget, below a separator; the label should say 
something like "%n plugged in devices are currently hidden"; the popup icon 
should change even when a hidden device is plugged in, with the popup not 
coming up. 

  --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

> * when an item is expanded, should the background remain painted? it might 
> make it more evident that the item is "open", and it would also allow a way 
> to 
> solve the next point, too. (and if the background remains, perhaps the 
> capacity bar should too?)

Now that I think some more about it... could you please explain more precisely 
what you mean by that?
If itemBackground stays on the open device, how can we move it to other devices 
when they are hovered?
Does that mean that we have to close it before selecting another one? 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

> just move it back to playground.

Done.
> 
> 
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

On Sunday 04 October 2009 20:57:11 Aaron J. Seigo wrote:
> On October 4, 2009, Jacopo De Simoi wrote:
> >  now, we believe that it is ready for merging into trunk. Of course we need
> >  your feedback first, so grab it, try it out and tell us what you think!
> 
> with options out of the way :) some thoughts on the plasmoid itself:
> 
> * it still says "Devices recently plugged in:" when there are no devices 
> listed. that's always seemed a bit odd. it should probably be replaced with 
> an 
> applicable label and the divider line removed in that case

The divider line was put there to avoid the bad "cropped" feeling of the 
scrollwidget when scrolled down; 
this is actually being addressed in the scrollwidget itself, with the nice 
shadows that now appear when the scrollbar is visible, so I believe that it can 
go back to having it getting along with the categories.
Still I don't know if it makes sense to have one also for the first category.

> * when an item is expanded, should the background remain painted? it might 
> make it more evident that the item is "open", and it would also allow a way 
> to 
> solve the next point, too. (and if the background remains, perhaps the 
> capacity bar should too?)

The background could  in principle stay, for the capacity bar there is an 
issue: there is no way to make a refresh signal 
"free space changed"; that's why we show it only on hover (and for the same 
reason it is like that for kfileplacesitem); 
 
> * each DeviceItem gets its own ItemBackground in case it has multiple 
> actions. 
> it would be nice if they could share the one ItemBackground between them as 
> only one at a time can be shown anyways so anything more seems like a waste; 
> it would also allow the hover effect to track with the mouse more effectively 
> when there are multiple DeviceItems in the list. 

I don't think I understand this; if we had one itemBackground shared by all 
items we would see the selection bar move from a device to another one. Do I 
understand correctly?

> * when there is more than one action, and an action is selection, perhaps the 
> device item should collapse back to closed. this does two things: it gives 
> some immediate feedback that an action was selected, it saves a second click 
> needed to collapse the item again. the assumption here is that the click 
> action is almost always needed once, which seems to fit the use cases pretty 
> clearly. for a single device item this might not matter, but if there are 
> multiple devices this could help avoid having to scroll more than needed or 
> click around a lot. 

Makes sense!

> 
> * in DeviceItem there is this line:
> 
> Plasma::IconWidget *actionItem = new Plasma::IconWidget();
> 
> where does this object get deleted?

good point.

> * in NotifierDialog there is a Plasma::Label ("category") and a 
> Plasma::Separator ("separator") created in 
> NotifierDialog::searchOrCreateDeviceCategory. they are deleted in 
> removeDevice, but that is only called when the source from the DataEngine 
> goes 
> away, the device is hidden by the user or resetDevices is called (e.g. after 
> configuration). it seems these will leak if the applet is simply closed

right.

> 
> * there is another separator created in NotifierDialog::buildDialog that has 
> no parent. 
> 
> * i committed a work around for a Qt bug that caused the capacity bar to 
> sometimes be shown in the wrong place (QGraphicsLayout strikes again!)
Excellent, thanks.

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: New devicenotifier moved to kdereview

[Jacopo De Simoi]

On Sunday 04 October 2009 12:36:49 you wrote:
> > **Please notice that there is a OLD devicenotifier in kdereview; make sure
> >  you grab the right one**
> 
> This is a joke right? Can't the plasma team make their mind up and remove the 
> old one if it's deprecated by this one?
> 

I should at least contact the author(s) of the previous refactoring, which I 
haven't seen recently; 
as soon as i make contact with him/them,  rest assured  the old one will be 
deleted.  

> So you didn't even try to make an effort? I'm sure kde-devel channel is full 
> of people that can point you into the right direction to get some i18n advice.

Will do that right now, particularly regarding the Messages.sh issues. Thanks

 Thanks
  --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

New devicenotifier moved to kdereview

[Jacopo De Simoi]

Dear plasma-devs, hardware-devs, core-devs,

  giucam and I, with some very-much-appreciated help by notmart have been 
lately quite busy porting the devicenotifier to use QGraphicsWidgets; the 
superficial look-and-feel is pretty much the same as the old device notifier, 
plus the notable improvement given by the features of giucam "big revamp", 
which you might have lately enjoyed in trunk. 
Many visual aspects have been polished along with some little new features here 
and there which have been added, which improve the user experience quite a bit 
in our opinion and make it even more consistent with plasma than it was before.
We spent quite some time trying to get rid of most bugs and as for now, we 
believe that it is ready for merging into trunk. 
Of course we need your feedback first, so grab it, try it out and tell us what 
you think!

trunk/kdereview/plasma/applets/devicenotifier-refactor

**Please notice that there is a OLD devicenotifier in kdereview; make sure you 
grab the right one**

To whom it may concert: there are a couple of new strings in the configuration 
dialog (which have been there since the big revamp, so ~1 month ago); I am 
i18n-challenged and I do not know how to notify these to the i18n-team; please 
let me know so that I can take all the necessary steps.

Thanks in advance.

  --J
 






___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Initial size and aspect ratio

[Jacopo De Simoi]

> > Speaking of which, it would be nice if a popupApplet had a way to
> >  /remember/ its preferred AspectRatioMode; in fact last time I checked,
> 
> it already does.
> 
Ah, the joys of trunk!
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Initial size and aspect ratio

[Jacopo De Simoi]

> > setAspectRatioMode(Plasma::IgnoreAspectRatio)
> 
> Yes I just found it in the qalculate source :-) I was looking for a 
> PopupApplet example.

Speaking of which, it would be nice if a popupApplet had a way to /remember/ 
its preferred AspectRatioMode; in fact last time I checked, once the 
popupapplet is docked in a panel, this property is lost, so that if we put it 
back on the desktop, we are left with a default KeepAspectRatio mode. 
I wonder which is the best/correct way to correct this issue.

 --J
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel