Add flags for setting icons, sink names

[OJFord]

For example:

polybar-pulseaudio-control --sink-icon='⇉ ' \
  --sink-name-from='device.description' --vol-icon-low='🔈 ' \
  --vol-icon-mid='🔉 ' --vol-icon-high='🔊 ' listen

Towards #26. I haven't added options for everything since I thought it'd be better to get some early feedback, and, frankly, these are the ones that I care about :smile:.

Sink nicknames from a pulseaudio property is a bit of a departure from the way it works setting within the file, but it seemed like it's be easier to use, and on my system at least has pretty usable results (to the point I almost want to suggest device.description as a default instead of numbers) - and perhaps there's a pulseaudio-wide way of overwriting descriptions (or other properties) that might be better than maintaining a separate list of polybar nicknames? Haven't looked into that yet.

In fact, it's surely possible, since I can change device.description & bluez.alias together for bluetooth devices by renaming them in blueman.

[marioortizmanero]

I like your idea about --sink-name-from, but what if, instead of applying it with setNickname at the beginning of the script, we read the property as a fallback before it's displayed in getNickname? That way, if a new sink is added we'll get its description as well.

[OJFord]

Yes that's actually a problem I encountered running it today - no description when I connected my Bluetooth earphones.

I was thinking the solution would be setting interval to something reasonable (and putting up with that as a maximum period of no nice name) - but I prefer yours. Coming.

[marioortizmanero]

Great! I think the base implementation is quite good already. If you have time, you can add the rest of the flags now (and update the README if possible) :+1:

[OJFord]

Done, and I realise now forgot the readme (sec) - though I confess I haven't tested them all (some I'm not set up to) but they were pretty straightforward with the parsing already there.

[OJFord]

Oops yes, sorry I missed the 'Usage' section. Not sure what you want to change in 'Useful icons' though?

[marioortizmanero]

The "Useful icons" format is made for the previous configuration method. So instead of ("🕨 " "🕩 " "🕪 ") it could be --vol-icons="🕨 ,🕩 ,🕪 ".

I meant something similar for the "Configuration" section too. Instead of SINK_NICKNAME_PROP and such it should be --sink-nickname-prop, because the previous one was meant for configuration by editing variables. Kinda like the "Usage" section but with more detailed explanations and in a table.

[OJFord]

Oh I see - I wasn't presuming to remove mention of the previous method (since it would still work).

Will do.

[marioortizmanero]

Yeah after this PR I don't expect anyone to configure the script by editing the top variables. Sorry if I wasn't clear with that. Having two ways to configure the script is a bit confusing IMO and using arguments is almost always better than editing the script anyway.

Yes, the Configuration section might be too repetitive now. Maybe it should be combined with the help message?

[marioortizmanero]

Also, are you still planning on releasing this on the AUR? If so, let me know when you do. We can add that to the README, and I could help you as an Arch user with a couple packages in there as well.

[marioortizmanero]

• Should usage be called when an action/option is unrecognized?
• Should we have a line length limit for the options help message?

I've made some changes I wanted to do. Let me know what you think.

[OJFord]

Also, are you still planning on releasing this on the AUR?

I would like it to be available there yes, happy to package it myself or I'll of course hold off if you'd like to maintain that yourself - you'll always know when there's a new release after all :slightly_smiling_face:.

Should usage be called when an action/option is unrecognized?

The reason I didn't was so that there's a single line of output when it's misused - reading nicely in polybar.

When I booted my PC just now for example, since I hadn't changed my main config since making these changes, polybar read 'Unrecognised option: --sink-icon'. If we printed the full usage polybar would just show the first line 'Usage: ...' which is less helpful. I suppose printing usage after the current output would work though (not tested).

Should we have a line length limit for the options help message?

Probably.. actually even the first one (autosync) is a little long compared to where e.g. git & grep wrap. Not sure if there's any pseudo-standard for help text specifically, but I doubt anyone would complain about 80ch.

[marioortizmanero]

I would like it to be available there yes, happy to package it myself or I'll of course hold off if you'd like to maintain that yourself - you'll always know when there's a new release after all slightly_smiling_face.

We could just co-maintain the package, if you want (I think that's possible in the AUR).

The reason I didn't was so that there's a single line of output when it's misused - reading nicely in polybar.

When I booted my PC just now for example, since I hadn't changed my main config since making these changes, polybar read 'Unrecognised option: --sink-icon'. If we printed the full usage polybar would just show the first line 'Usage: ...' which is less helpful. I suppose printing usage after the current output would work though (not tested).

Fair enough.

Probably.. actually even the first one (autosync) is a little long compared to where e.g. git & grep wrap. Not sure if there's any pseudo-standard for help text specifically, but I doubt anyone would complain about 80ch.

80 characters is my goto as well. It looks a bit stretched because of how long the arguments are, not sure what you think about that:

It uses a single echo from now on because it's much easier to edit and because of the character line at 80 chars. I've also updated the message according to your review, but I'm not sure if that's exactly what you wanted. Let me know otherwise.

I think we should also change the default value so that it's between [ instead of (. The latter might be thought of as part of the explanation, while the former stands out as something different. This is just nitpicking, though.

[OJFord]

We could just co-maintain the package, if you want (I think that's possible in the AUR).

Ah yes, so it is. :+1:

:+1: for single echo, I was tempted to do that too.

Screenshot looks ok to me, maybe gets a little hard to read at the bottom over several lines, but then spaces around that and not the others might look weirder. (Also nitpicking though!) Think I agree about [] defaults.

[marioortizmanero]

I've ended up using an indentation of 2 spaces instead of 4 for the help message, which gives just a bit of space more (not sure what else can be done). I'll merge this and we can discuss the PKGBUILD at #35.

Thanks a lot for your help with this PR!

[OJFord]

Awesome, thanks!