rofi: add alternatePattern option#2193
Conversation
gibbz00
commented
Feb 8, 2026
gibbz00
commented
- I certify that I have the right to submit this contribution under the MIT license
- Commit messages adhere to Stylix commit conventions
- Changes have been tested locally
- Changes have been tested in testbeds
- Each commit in this PR is suitable for backport to the current stable branch
stylix-automation
bot
added
topic: home-manager
There was a problem hiding this comment.
- Changes have been tested locally
Could you provide screenshots for the {before,after} * {dark,light} matrix of this PR?
- Changes have been tested in testbeds
The /modules/rofi/ module has no testbed. The closest thing might be /modules/wofi/testbeds/wofi.nix.
Sure 😊 With ayu-{dark,light}: Dark Alternating
Dark
Light Alternating
Light
Apologies, misunderstood it as "I've run the test and make sure they still work." |
modules/rofi/hm.nix
Outdated
| mkLiteral "rgba ( ${r}, ${g}, ${b}, ${opacity'} % )"; | ||
| mkRgb = mkRgba "100"; | ||
| rofiOpacity = toString (builtins.ceil (opacity.popups * 100)); | ||
| mkAlternate = alternate: fallback: if cfg.alternatePattern then alternate else fallback; |
There was a problem hiding this comment.
Consider renaming and swapping the function arguments as follows:
| mkAlternate = alternate: fallback: if cfg.alternatePattern then alternate else fallback; | |
| mkAlternate = base: alternate: if cfg.alternatePattern then alternate else base; |
Do not forget to swap the argument order for all callers.
There was a problem hiding this comment.
Any reason for why that is better? My reasoning was:
"If alternate; use this color, otherwise fall back to this"
mkAlternate (mkLiteral "@foreground") normal-foreground
There was a problem hiding this comment.
Any reason for why that is better? My reasoning was:
"If alternate; use this color, otherwise fall back to this"
Ideally, implementation details do not leak. The alternate: fallback: argument order reads nicely when the implementation reads as alternate: fallback: if isAlternate then alternate else fallback, but would be contrived for alternate: fallback: if isNotAlternate then fallback then alternate. My base: alternate: argument order establishes the base case first and then the exception. The fact that the exception is enabled by default is an implementation detail. This interpretation is obviously reversed when considering the alternate state as the base case.
Both interpretations are fine, although I prefer the base: alternate: order, but the fallback term incorrectly implies faulty behavior.
There was a problem hiding this comment.
i tend to agree with NAHO here, but it's only a slight preference and would be fine merging either way.
3 participants
