search

tobi-wan-kenobi / bumblebee-status

bumblebee-status is a modular, theme-able status line generator for the i3 window manager.
https://bumblebee-status.readthedocs.io/en/main/
MIT License
1.2k stars 229 forks source link

pulsectl: devicename.lower() may be a breaking change #989

Closed dronte-b closed 9 months ago

dronte-b commented 9 months ago

Bug Report

Description

Affected module: pulsectl Version used: latest master

Recent commit introduced this line https://github.com/tobi-wan-kenobi/bumblebee-status/commit/9e6e656fa87eb9e5ae45a790bbd427b88de5b186#diff-f250fb402e42a6d1dbdfbc9039d030ea109009df4400c5fd0744310f8d8176d9R113

What happened to me: I had a config entry like pulsectl.alsa_output.usb-Logitech_Logitech_USB_Headset-00.analog-stereo=Headset

And it worked before this commit, but now renaming looks for lowercased name of the device int self.parameter which contains the parameters as they are given, without specific lowercasing. So the remapping fails

tobi-wan-kenobi commented 9 months ago

Yes, thanks a lot, this is a bug. Changes generally should be backwards-compatible.

TheEdgeOfRage commented 9 months ago

The weird thing is, I added that .lower() because in my case the bumblebee config parser converts the device name to lowercase in the config map. The pulsectl lib still returns the properly cased one, so for me it doesn't work unless I have the .lower() call on the name returned from pulsectl.

I'm not sure whats different in my case and why my config is getting parsed as lowercase :thinking:

PS. Sorry for breaking this :sweat_smile: I assumed it would be broken for everybody the same way it was for me

tobi-wan-kenobi commented 9 months ago

Strange, I tested setting via "-p" and there, casing is maintained. Do you use a config file? Maybe that behaves differently (which would be abbug, IMHO)

Generally, I typically prefer case sensitivity.

TheEdgeOfRage commented 9 months ago

Yeah, I have it in a toml config file. It probably gets mangled during parsing, or maybe toml doesn't have case-sensitive keys :shrug: FWIW, I'm also for case sensitivity, if it can be implemented here. Should I open a separate issue for this?

tobi-wan-kenobi commented 9 months ago

yes, please. that is probably the best course of action.

thanks for finding this!