Tracker: Patches

5 volumealsa plugin with Low/Mid/High/Mute icons - ID: 3103192
Last Update: Comment added ( gilir )

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


gilles bernard ( lordikc ) - 2010-11-04 14:25:49 PDT

5

Open

None

Nobody/Anonymous

None

None

Public


Comments ( 5 )

Date: 2011-12-24 06:39:19 PST
Sender: gilir

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


Date: 2010-11-07 12:36:25 PST
Sender: lordikc

New icons + same behaviour with or without theme.


Date: 2010-11-05 08:29:10 PDT
Sender: martyj19

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.


Date: 2010-11-04 23:56:24 PDT
Sender: lordikc

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 ?



Date: 2010-11-04 15:30:59 PDT
Sender: martyj19

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.


Attached File ( 1 )

Filename Description Download
volumealsa.patch.tgz Patch + icons Download

Changes ( 3 )

Field Old Value Date By
File Added 392528: volumealsa.patch.tgz 2010-11-07 12:35:32 PST lordikc
File Deleted 392288: 2010-11-07 12:34:42 PST lordikc
File Added 392288: volumealsa_icon.patch.gz 2010-11-04 14:25:52 PDT lordikc