#367 volumealsa plugin with Low/Mid/High/Mute icons

open
nobody
None
5
2010-11-04
2010-11-04
gilles bernard
No

Hi,

Presently, the volumealsa icon has only two icons corresponding at volume muted or not.
The attached patch adds icons for volume low, mid and high the same way the volume plugin does.

I think it would be valuable to have this feature in volumealsa plugin.

Regards,

Lordikc

Discussion

  • martyj19
    martyj19
    2010-11-04

    The idea is worthwhile, but you haven't done anything in your proposed changeset to ensure that the icons you plan to use are going to be available at runtime. We normally ship in the tarball and install in /usr/share/lxpanel/images what we are going to use and look for them as a fallback to whatever is available in the selected icon theme.

    Also, it is a very good idea to get into the habit of formatting your code in the existing style of a module when you submit a patch to an open source project, if you want the developers to take enough interest in your work to apply it. You are of course free to write code in your own projects any way that pleases you.

    You need not revise your patch to incorporate these items. The change is simple enough and it may be adopted in a future release.

     
  • gilles bernard
    gilles bernard
    2010-11-05

    I rely on the standard icons present in the theme. The present code already rely on some of them : audio-volume-muted and audio-volume-high. It falls back to images shipped with lxpanel.
    I just extend to the use of two other icons presents in the audio-volume serie : volume medium and volume high.
    The code still fall backs to default images in case these are not present.
    So the behaviour is : If icons are in the theme, use them for 4 states (off,min,med and max). Otherwire, fallback to default and 2 states (off or on).

    About code formatting, the only difference I can see is comments on the right of the if statement. Is that is ?

     
  • martyj19
    martyj19
    2010-11-05

    I don't think it would be a good idea to have the behavior change that radically depending on what icons are present in the theme. You would get bug reports about that instead. I would rather see the same behavior in all cases either with the icon theme's icons or with our supplied icons.

    On the formatting, I was referring to missing spaces around the operators and two space indent rather than four. I tend to use paragraph comments above a block of code to explain that block and set each block off with a blank line. I do use end of line comments occasionally but usually only when something is there because of a bug or workaround that would not be obvious and I would not want someone to come along later and disturb. It seems to me that the comments you have are overcommenting, because the code is not that difficult to follow. I feel like less experienced people tend to overcomment because to them everything is difficult. But that's only one person's viewpoint on the commenting style.

     
  • gilles bernard
    gilles bernard
    2010-11-07

    Patch + icons

     
    Attachments
  • gilles bernard
    gilles bernard
    2010-11-07

    New icons + same behaviour with or without theme.

     
  • gilir
    gilir
    2011-12-24

    Patch committed to lxpanel git master, thanks for your contribution :)