Review of "Alsa Mixer" version 6

Details Page Preview

Control Alsa master volume from status menu. Requires 'amixer', provided by alsa-utils package.

Extension Homepage
https://github.com/tghosgor/gnome-shell-extension-alsamixer

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental)

Shexli has not produced a result yet.

All Versions

Previous Reviews on this Version

fmuellner rejected
No, that is not what I suggested - the default volume menu is already hidden as necessary (at least assuming the code is working as intended - if not, a bug report is more appropriate than a workaround in an extension), so you should not need to touch it at all. However if pulseaudio is running (e.g. the default volume indicator/menu is visible), *your* indicator and menu should hide themselves. Regarding "//expensive because of GLib.spawn_command_line_sync - only used on enable anyway" - if users use amixer themselves, the actual volume would get out of sync with what your menu claims, no? (Rejecting because of the first point - you may end up showing the default volume menu when it's not supposed to be visible)
tghosgor posted a review
I am using Arch and GNOME without pulseaudio installed. The indicator is hidden but the volume menu is not. It's been like this for some time. I didn't know this was a bug. I will stop modifying the mixer in the menu then. About the synchronization between amixer and the volume indicator, yes it would go out of sync. I don't know any way to "watch" for amixer value apart from sync/async polling it every 2 seconds. Is there any other way?
fmuellner posted a review
Oh right, looks like we only do that for the input slider - arguably that's still wrong, but I guess it's OK for you to show/hide the menu then. However you should still make sure to show/hide your indicator/menu as well!