Display Apple symbols in keybinding menus - #307
mantisdotdev wants to merge 2 commits into
Conversation
714f91a to
43d3dd1
Compare
scottjones
left a comment
scottjones
left a comment
There was a problem hiding this comment.
Thanks for this — the alignment work is careful, and the tests are real: they render through stubbed hyprctl/tmux/herdr and the new record test actually drives format_display_records. All three test files pass on this branch and omarchy commands --check is clean.
One blocker, then two nits.
bug: the glyph substitution is unconditional in all three menus. This tree is heading toward serving x86 upstream hardware too (#331 adds omarchy-hw-aarch64 and omarchy-hw-apple-silicon), and on a PC keyboard SUPER shown as ⌘ is just wrong. Please gate the substitution on omarchy-hw-apple-silicon (or an equivalent keyboard check) so non-Apple machines keep the plain names. Inline comments mark the three spots.
nit: padded() is copied into the tmux and herdr scripts, with a third inline copy in format_display_records. One shared shape would be easier to keep in step.
nit: the rewritten assertions in keybindings-menu-test.sh and menu-keybindings-fkeys-test.sh will conflict on every upstream merge of those files. If the glyphs are gated, the existing assertions can stay as they are and a Mac-only test can cover the symbols.
| count = split(display, parts, /[[:space:]]+/) | ||
| display = "" | ||
| for (i = 1; i <= count; i++) { | ||
| if (parts[i] == "SUPER") parts[i] = "⌘" |
There was a problem hiding this comment.
bug: this runs for everyone. Please wrap the symbol mapping so it only applies when omarchy-hw-apple-silicon (from #331) succeeds; otherwise upstream x86 users see ⌘ for their Windows key.
| if (is_modifier && part == "C") return "CTRL" | ||
| if (is_modifier && part == "M") return "ALT" | ||
| if (is_modifier && part == "S") return "SHIFT" | ||
| if (is_modifier && part == "C") return "⌃" |
There was a problem hiding this comment.
Same gate needed here: the tmux menu should only switch to ⌃/⌥/⇧ on Apple hardware.
|
|
||
| for (i = 1; i <= count; i++) { | ||
| text = text (text == "" ? "" : " + ") toupper(parts[i]) | ||
| part = toupper(parts[i]) |
There was a problem hiding this comment.
nit: part is not in the parameter list, so awk makes it a global. The rest of this file declares locals as trailing parameters (key_text(key, count, parts, i, text)), so part belongs there too.
|
The existing blocker remains: modifier glyph substitution is unconditional. Gate Apple symbols on Apple Silicon and keep the current plain labels and assertions for x86. |
|
Apple modifier symbols render correctly in the main, Tmux, and Herdr keybinding menus. Arrow columns remain aligned, long labels stay intact, and no clipping or overlap is visible. Good to merge. |
wesleygrimes
added
the
cherry-pick-later
4 participants
Summary
⌘,⌥,⌃, and⇧in the main, Tmux, and Herdr keybinding menus.Verification
git diff --checkpass.