Caps Lock indicator colors not used

JohnMertz commented 9 months ago

System/Version

Version: 1.7.2 Distribution: Fedora Sericea (Sway spin of Silverblue) 39

I have not yet tested from Git, but will follow up with that when I have the time. However, the blame for the lines mentioned below shows that it has not changed since the feature was added in https://github.com/swaywm/swaylock/commit/1e7696fceba795e8f67cc11fdc05d44d8d6fcf69

Problem

This is a fairly trivial bug as far as the impact on the user and I suspect probably in what is needed to fix it.

The actual problem is that the config values for caps-lock-key-hl-color, caps-lock-bs-hl-color and ring-caps-lock-color don't seem to be used. When CapsLock is active, the indicator just uses the regular colors for key-hl-color, bs-hl-color and ring-color, respectively.

Steps to replicate

Have a config file with values for:

caps-lock-key-hl-color=AAAAAAFF
caps-lock-bs-hl-color=AAAAAAFF
ring-caps-lock-color=AAAAAAFF

which differ from the values for:

key-hl-color=BBBBBBFF
bs-hl-color=BBBBBBFF
ring-color=BBBBBBFF

Run swaylock type/backspace with and without CapsLock enabled. The former values will never be displayed.

Notes

At a quick glace, the conditions to select these colors look like:

https://github.com/swaywm/swaylock/blob/b63aaffcd17b4115aa779e49016c6c4dcf06ecd9/render.c#L277-L281

I suspect that one of the two conditions to confirm the state is incorrect. An alternative might be that the correct color value is never assigned or is overwritten by the regular colors.

Note that the text-caps-lock-color value is used correctly.

I haven't looked at this beyond locating the above LOC. I can probably fix this, but won't likely have time until the weekend. Just thought that I'd report before opening a PR in case someone else can get to it quicker, correct my understanding, or point me in a better direction.

budimanjojo commented 9 months ago

I can confirm this also happens with https://github.com/jirutka/swaylock-effects. Maybe you can also create a PR there too (after you finished with the PR here) if you want? Or I will be glad to help with the PR in there.

budimanjojo commented 9 months ago

I think I know why, it's by design: https://github.com/swaywm/swaylock/blob/b63aaffcd17b4115aa779e49016c6c4dcf06ecd9/render.c#L21-L33

Based on my understanding from the code, the caps-lock colorset is only used when --indicator-caps-lock flag is set to true. The text color of capslock is used even if the flag is not set which explains why the text color is being used.

So this is expected result based on the code, but I would suggest to not change the text color of capslock when --indicator-caps-lock flag is not true because it confuses people.

budimanjojo commented 9 months ago

I suggest line 24-39 in the snippet above to be removed, not sure why they're there. Is there any reason for that?

swaywm / swaylock