Skip to content

Give binds a stable id to key on - #12557

Open
linyiru wants to merge 6 commits into
omacom:quattrofrom
linyiru:bind-stable-id
Open

linyiru wants to merge 6 commits into
omacom:quattrofrom
linyiru:bind-stable-id

Conversation

@linyiru

@linyiru linyiru commented Sep 19, 2026 •

Copy link
Copy Markdown

Pilot for #12404, which argued that a bind's description is doing work no display string should do. This is the smallest change that separates the two names, and it changes no behaviour on its own.

o.bind gains opts.id, defaulting to the description. The menu's Lua bind cache keys its dispatcher and arg on the id. The four web app binds that focus an open window name their id outright, because that string is matched against the window's own class or title, so it belongs to the web app rather than to us.

What does not change

The menu keeps its own scan of the Lua config, because Hyprland reports every Lua bind as dispatcher: __lua and reports no id at all. Running that scan over a stock config before and after:

229 rows before, 229 rows after
the id column removed, diffed against before   →  byte identical
rows where the id differs from the description →  0 of 229

So every bind Omarchy ships resolves exactly as it did, including the four web app binds whose command now comes from an id that happens to spell the same thing. keybindings_cache_key therefore does not need its version bumped: an upgraded install serving a cached record set is serving the same records the new code would write.

The one lookup that keeps reading display text

Hyprland gives the menu a modmask, a description and usually a key. A key plus a description is enough to find the bind again, and everything downstream of that keys on the id.

The loop-generated workspace binds are the exception: Hyprland reports neither key nor keycode for them, so their description is the only thing on offer, and the key they display is recovered entirely from the source cache. 59 binds in my session arrive that way, around a quarter of the cheatsheet. That lookup cannot key on an id, because finding the id needs the key it is looking for. It stays where it was, now with a comment saying why, and it is the only place left where renaming a description could move anything.

Three pairs in the tree already share a modmask and a description (Switch media source, Browser, Next window in group). All three pairs dispatch the same thing as each other, and the maps that could confuse them include the key, so nothing there changes either way.

What the existing tests caught

Worth recording, because it is the failure mode a reviewer would look for. My first version guarded the dispatcher lookup on the id being found:

if [[ $dispatcher == "__lua" && -n $id && -n $key ]]; then

keybindings-menu-test.sh failed on "chords whose dispatch is unknown stay apart". A bind the cache has nothing for used to fall into that block and come out with an empty dispatcher; with the guard on the id it skipped the block and kept the literal __lua, which reads as two unresolved binds agreeing about what they run, and the menu merged two rows that should have stayed apart. The guard stays on the reported fields, and the blanking is now commented so the next person does not tighten it again.

Tests

New test/shell.d/hyprland-bind-id-test.sh, 9 assertions: the id defaults to the description, a declared id survives and leaves the description alone, a { webapp, focus = true } bind matches its window by id and not by description, a toggle bind carries a declared id, every bind the scan reports answers to an id, a bind written straight against hl.bind still gets one, and the four web app binds in the tree each name their own id so a fifth cannot quietly inherit one.

./test/cli and ./test/shell were run on the branch and on quattro on the same box. 8 assertions fail, the same 8 on each side and no others, all of them checking system state a plain checkout does not have (half of them name a missing companion omarchy-pkgs clone). So the branch is even with its base.

One question

For those four binds, the alternative shape is focus = "WhatsApp" instead of { id = "WhatsApp" }, reusing the string form command_from already accepts for { launch, focus } binds ({ launch = "obsidian", focus = "^obsidian$" }). That would keep the window pattern out of the id entirely, at the cost of the pattern and the id being two separate things to keep in step. I went with the id because #12404 is about having one stable handle per bind, but I am happy to switch.

Found while writing the test, unrelated

The scan cannot see a user's own binds at all. It dies partway through Omarchy's defaults, at default/hypr/qconsole.lua:123 (attempt to compare number with table, because the stub hands back a table where a monitor scale is expected), and the pcall around it swallows that silently, so require("hypr.bindings") never runs. Anything in ~/.config/hypr/bindings.lua reaches the menu without a resolved dispatcher. Identical on quattro, so it predates this change; noting it here so it is not read as fallout from this one. It already has fixes in flight rather than needing a new report: #7564 is the one carrying a test that fails on quattro, and it and this branch merge without a textual conflict while breaking each other's tests, which is measured in a comment below.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Pilot-side report plus four remarks, from the ru pack (we run 254 translated binds live and now emit explicit ids on all of them — details at the end).

1. Vote: { id }, with mechanics. Under focus = "WhatsApp" the id stays defaulted (= description). Translate the label and the id floats: joins, precedence and round-trip start depending on translation again for exactly these four binds, while focus stays intact. That is half the breakage class this PR removes, narrowed to four binds. Under { id } there is one stable handle for all five roles, and the label is fully free. The "two things to keep in step" argument also cuts the other way: focus = leaves three entities (label, defaulted id, pattern) where { id } leaves two (id, label). Our pack keys on ids either way, but only under { id } does "never these four ids" cover everything.

2. The default makes safety opt-in — ratchet it. "Every bind answers to an id" passes trivially through the default, so "a translated bind carries an explicit id" is enforced by nothing. Proposal in test form: pin the count of binds relying on the default (227 − 4 explicit today) and fail on growth. Old rows migrate gradually, new holes cannot appear silently.

3. Six lines of insurance. XF86AudioPause/Play (modmask 1), RETURN/B (65), TAB/mouse_down (72) share modmask+description today. Safe now (same dispatchers, key in the map keys), but the PR itself names the detonation condition (a move into a loop or code: form) — and our pack mass-produces code: duplicates of letter binds, so we stand closest to that mine. Explicit ids for the six fit the spirit of the branch.

4. qconsole gap. Reproduced on a live user config (DEBUG=1, same qconsole.lua:123: attempt to compare number with table) — user binds never reach the scan. Happy to file it as its own issue with our log, or leave it to you.

Our side, done: our generator now emits { id = "<stock EN description>" } on every generated bind (254/254, verified live, zero runtime change — Hyprland ignores the key as measured). Main-loop ids are stock descriptions verbatim; loop ids mirror the stock loop descriptions (Switch to workspace N, Move window to workspace N, Move window silently to workspace N, Switch to group window I, Bar panel P), so they coincide with future stock defaults. The day this lands, our stream resolves by id with no rework — except the four matcher rows, which stay skipped until the { id } vs focus = question settles.

@WokoFlipper

Copy link
Copy Markdown

Follow-up from the ru pack pilot: stock clipboard under a Cyrillic layout is broken, with a precise error. Filing here since it is the same keysym-vs-layout class your adjudication measures (Hyprland 0.56.2 efb5099, omarchy 4.0.4-1).

Repro: press SUPER+C (stock Universal copy, dispatcher send_key_state({mods="CTRL", key="C"}) + 50ms down/up split) under the RU layout. Result: the command does not execute, and two desktop notifications appear: Runtime error in lua … send_key_state: key not found. Under EN the same press works. No coredump, no journal crash — the failure surfaces only as the lua runtime notification, one per send call (down + up), which is why users report "runtimes falling" in plural.

Root: the key name resolves through the active keymap, and there is no C in a Cyrillic layout. So every keysym-named injection is layout-fragile by construction; the 50ms down/up split then doubles the visible failure.

Workaround, proven live on our box: dispatch with X11 keycodes instead of names — key = "code:54" fires clean under EN and RU, zero notifications. Our generator now emits C/V/X this way (Russian labels, explicit {id}, no code: duplicate binds — dups double-fire under EN, we measured that separately). 257 binds, user-tested copy/paste/cut in both layouts and in terminals.

Ask: layout-independent injection in send_key_state (accept keycodes), or at least a documented statement that keysym-named injection is EN-layout-only. Happy to re-run any measurement on our RU rig.

@omarchybot omarchybot added the enhancement New feature or request label Sep 27, 2026
@linyiru

linyiru commented Sep 28, 2026

Copy link
Copy Markdown
Author

Rebased onto current quattro just now. One line of adjacency with #13521, which landed inside the same function this PR renames an argument of; nothing in the change itself moved. Answering your four, then the clipboard report separately, because I do not think it belongs on this PR.

1. { id }. Agreed, and your "three entities versus two" reading is the argument I did not have. Under focus = the id keeps defaulting, so the four binds this PR exists to protect are precisely the four left exposed. Nothing in the branch changes; the question is open for the maintainers rather than for us.

2. The ratchet: the count is the wrong thing to pin. Measured on the rebase. default/hypr/bindings/ went from 193 o.bind/o.bind_toggle call sites to 195 between this PR's original base and current quattro, nine days and six commits touching those files, with 0 explicit ids on either side. A pin on "binds relying on the default" fails on both of those additions, for reasons with nothing to do with translation, and whoever hits the failure has no way to tell a real hole from an ordinary new bind. One unit note while I am here: 227 counted rendered rows, which the loop binds expand into, and declared call sites are what a test can actually read off the source.

What is enforceable is a ratchet on the class that is dangerous rather than on all of them, and the test already ships one: every bind carrying webapp = ... with focus = true must also carry id = , checked against the source tree, so a fifth cannot quietly inherit an id that gets matched against a window title. That fails only when someone adds a genuinely load-bearing bind, which is the signal worth having. Generalised, it reads "any bind whose dispatcher consumes the id declares it", and today that set is exactly those four.

The translation-time form you want, "a translated bind carries an explicit id", can only be written by the layer that knows which binds it translated. That is your generator's own test, and 254/254 says you have already written it.

3. The six lines cannot attach where the mine is. I went to add them, then read the two lookups, so here is the reading instead of a patch.

The collision lives in LUA_BIND_KEY_MAP, written at bin/omarchy-menu-keybindings:85 as [modmask,description] and read at :318. It carries no key in its own key because its entire job is to recover the key for binds Hyprland reports without one. LUA_BIND_ID_MAP is read five lines later, at :323, keyed [modmask,description,key], so it cannot collide; and an explicit id is that map's value, never anyone's key. Line 318 runs before any id exists.

Which is the structural part. What arrives from Hyprland for those binds is a modmask and a description and nothing else. If two binds share both, the report is ambiguous at the source, and no id scheme downstream can disambiguate it, because the id was never in the report. Distinct ids would help if the ambiguity were in our tables. It is in Hyprland's output.

What would defuse it is a few lines at :85: notice a second write bringing a different key and blank the entry, so an ambiguous bind recovers nothing instead of recovering the other bind's key. Today that is unobservable, since all six report their own key and never read the map, and that is also why I cannot test the detonating path from here. It needs a live session holding a bind that shares modmask and description with another and reports no key. Your pack mass-produces code: duplicates of letter binds, so you are the one party who can build that deliberately. If you can get one of the six into a loop or code: form on your rig and tell me which key the menu then prints for the other one, that settles whether the guard earns its own PR.

4. qconsole. Mine to file, and thank you for the offer. Reconfirmed on current quattro today: a probe bind in ~/.config/hypr/bindings.lua is absent from all 229 rows, and DEBUG=1 prints the same qconsole.lua:123. Two independent configs is most of what that issue needs, so I will link your reproduction; add your log there if it shows anything mine does not.

For the record, the numbers moved with the rebase. The scan now emits 229 rows on each side instead of 227, still byte identical once the id column is cut, still 0 of them with an id differing from its description. Your 230 on a newer tree is the same measurement one step further along.

On the clipboard report

Different subsystem from this PR: default/hypr/bindings/clipboard.lua, not the menu, and nothing about it should wait on this branch. Please file it. Your repro reads complete already, and you have the only rig that can run it.

What I can add from the source, which widens it: four stock binds are in that class, not one. send_shortcut_once (clipboard.lua:8) names its key as a string and every caller hands it a Latin letter, so Select all (SUPER+A), Universal copy (SUPER+C), Universal paste (SUPER+V) and Universal cut (SUPER+X) are all layout-fragile by the same construction. Copy and paste also have a second, terminal-only path sending CTRL SHIFT plus the same letter, so a terminal fails on a different line than a browser does and a fix has to cover both. The 50 ms oneshot at :12 is what doubles the notification, exactly as you read it.

One question before code: becomes the recommended shape, which I cannot answer from this machine. The two halves would then resolve through different maps: the bind side (SUPER + C) is a keysym name, the injection side a physical position. Under RU that difference is working for you, since there is no Cyrillic C and yet the bind still fires, which says Hyprland is not resolving binds against the active layout the way it resolves injection. Under a primary Dvorak or Colemak layout I would expect the two to disagree: X11 54 is the physical key that spells c on QWERTY and j on Dvorak, so the application would receive Ctrl+J while the bind still fires from wherever c lives. If they do disagree, the ask changes shape, from "accept keycodes" to "resolve the injected keysym through the same map the bind used", with keycodes kept as the explicit escape hatch. That is one press on your rig with a second layout added, and it decides which fix to ask for.

@linyiru

linyiru commented Sep 28, 2026

Copy link
Copy Markdown
Author

Correcting something I wrote in the body and repeated to @WokoFlipper above: I said the qconsole.lua:123 gap wants its own issue and that I would file it. I searched before filing, and it does not want one.

Seven open PRs already fix some part of that scan: #7564, #8876, #12712, #12916, #13276, #12155 and #11733. Three open issues cover the runaway lua half of it (#7025, #12052, #12124), and the abort half is documented inside #7564 and #13276 rather than in an issue of its own. @alinuxfan's report on #7564 measures a worse symptom than I did: a user's o.rebind of a default keeps the default's description, so the menu does not merely lose the row, it dispatches the default's command while the real key does the right thing. #7564 is also the only one of the seven whose test fails on quattro. So an eighth artifact describing the same defect helps nobody, and I have put what I measured on that PR instead of here.

One finding does belong here, because it is about this branch. #7564 and this PR merge with no textual conflict and still break each other's tests. This branch adds the id column as field 3 of the scan output, which breaks that PR's two fixed-string row assertions; that PR rewrites the heredoc to timeout 10 lua <<'LUA', which breaks how hyprland-bind-id-test.sh extracts the scan. Measured on the merge: 1 ok / 1 not ok and 4 ok / 1 not ok respectively, with keybindings-menu-test.sh unaffected at 16/16. Both are a one-line fix, and I will make them here if #7564 lands first.

@WokoFlipper, the same search says your clipboard report is covered too, and more usefully than by a new issue. #12570 is your bug with the Russian layout named in the title; #7027 and #7371 are the same failure reported earlier. The two open fixes are the two shapes I asked you to choose between: #9130 sends XKB keycodes 38/54/55/53 for A/C/V/X, which is your code:54 workaround, and #13007 resolves the letter against the primary layout instead, which is the "use the map the bind used" alternative. #9130 says outright that no live non-Latin session was available to its author. You have one, and adding a Dvorak layout to it would separate those two PRs on the one point where they disagree. That measurement is worth considerably more on those two threads than another report of the bug.

@WokoFlipper

Copy link
Copy Markdown

Measurements posted as requested (see the two clipboard PR threads): hardcoded QWERTY keycodes agree with primary-layout resolution exactly when the primary is QWERTY (our live RU-secondary setup confirms daily), and diverge on Dvorak primary (54/55/53 hit j/k/q; correct are 31/60/56). So #13007 is the general shape, #9130 the QWERTY-only special case.

On the merge conflict you found (id column vs #7564 heredoc): happy to take the one-line fix on our side if #7564 lands first — say the word.

@linyiru

linyiru commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

@WokoFlipper the follow-up branch exists: linyiru:menu-sort-by-bind-id, one commit on top of this one.

git fetch https://github.com/linyiru/omarchy menu-sort-by-bind-id && git checkout FETCH_HEAD

It takes the id through parse_binding_records as a fourth field and hands both of the decisions you named over to it. Pairing is keyed on id SUBSEP dispatcher SUBSEP arg instead of on the label, so a translated pair still merges; prioritize_entries classifies on the id, so a translated row keeps its tier instead of falling into the default 50 with every other translated row. Two classifications stay on the chord on purpose, because they are about the chord and translation cannot reach one: the XF86 media-key tail, and the second browser chord. The id comes off again before the sort, so a cached record is still the three fields it always was and the cache key stays at v14.

Read this part before you run it, because on its own the branch will look like it does nothing for you.

While testing I found the menu keeps its own Lua scan of your config, and that scan dies partway through the shipped one. default/hypr/omarchy.lua:18 requires qconsole, qconsole.lua:123 compares mon.scale > 0 where the scan's stub makes mon.scale a table, and the pcall around the whole scan swallows it. 229 default binds have registered by then. Everything after line 18 is lost, and config/hypr/hyprland.lua:21 requires hypr.bindings after line 14, so your own ~/.config/hypr/bindings.lua is never scanned at all. No id, no dispatcher recovery, on this branch or before it.

So I put the fixture from my own tests where a real user would put it, in ~/.config/hypr/bindings.lua, and ran the menu off this branch:

# two chords, same id "Close window", description translated
# shipped load order:
SUPER ALT + Q                       → 關閉視窗
SUPER ALT + W                       → 關閉視窗

# same binds, load order changed so the scan reaches them:
SUPER ALT + W / SUPER ALT + Q       → 關閉視窗

The branch is necessary but not sufficient. If your 254 descriptions live in ~/.config/hypr/bindings.lua, the scan never sees the ids they carry, the parser falls back to the description exactly as it does today, and you get the split rows and the flattened order you already have. That is not the branch failing; it is a second defect sitting in front of it. I would rather hand you that than have you spend an evening bisecting my commit.

Correcting myself, and it is good news for you: the scan gap is already fixed in an open PR. @playGitboy opened #13276 on 2026-09-25 against exactly this, same error at the same line, with more measurement than I had and a fix that teaches the stub to survive being read. llstrk has reviewed and confirmed it. So do not reorder your config, which was the workaround I was about to suggest and which breaks o.rebind of a default anyway. Stack the two instead:

gh pr checkout 13276
git fetch https://github.com/linyiru/omarchy menu-sort-by-bind-id
git merge FETCH_HEAD

They merge with no conflict, which is worth stating because llstrk found that five of the six other PRs touching that scan do conflict with #13276. Measured on the combination, with two chords declaring the same id = "Close window" and a translated description, both in ~/.config/hypr/bindings.lua, shipped load order, nothing reordered:

tree rendered cached dispatch
this branch alone two rows, Q above W dispatcher=[] arg=[]
plus #13276 SUPER ALT + W / SUPER ALT + Q → 關閉視窗 dispatcher=[exec] arg=[true]

So #13276 is what makes this branch do anything for you, and together they do what you asked for.

I had opened an issue for the scan gap before finding #13276. That was a duplicate and my own mistake: I had searched this repo for prior art on that exact defect last week, found the cluster of PRs around that scan, and publicly said I would not file it. I then filed it today without re-running the search. It is closed and pointed at #13276.

On the branch itself: four new assertions in test/shell.d/keybindings-menu-test.sh, and I reverted each of the three production hunks on its own to confirm each assertion actually fails without the code it claims to test. That caught one of my own assertions passing for the wrong reason, off an alphabetical tie-break rather than the rule. Ordering and chord merges on a real 254-description config are the part I cannot produce here, so I am glad you are running it.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Stack test results as promised. Tree: PR #12557 base + #13276 fix + id-sort commit (clean applies, no conflicts) — all 10 keybindings-menu tests pass, including the 4 new id assertions.

Clean real config as requested (generated 257-bind block only, personal binds stripped): https://gist.github.com/WokoFlipper/7b46567bfb624c61ba928fe2572234b2

One finding while stacking: the id-sort commit alone on #13276 is NOT enough — the id plumbing (o.bind/scan emission) lives in the #12557 base. Without it $id stays empty and translated pairs still split. The full three-layer stack is the working combination.

Live render on this config via the menu frontend still pending — will report pairing visually once run.

A bind's description is display text, and three things use it as an identity:
the menu's Lua bind cache, the key that cache recovers for loop-generated
binds, and the window pattern a web app bind focuses by. Renaming a bind moves
behaviour along with the label.

Add opts.id, defaulting to the description so nothing changes for a bind that
needs no second name, and key the menu's dispatcher and arg cache on it. The
four web app binds that focus an open window name their id outright, since that
string is matched against the window's own class or title.

Hyprland reports no id, so a reported bind is still matched back to one through
its description; that lookup and the key recovery beside it are now the only
two places display text is load-bearing.
@linyiru

linyiru commented Sep 30, 2026

Copy link
Copy Markdown
Author

Thank you for stacking it, and for the config. Both are more than I asked for.

Your finding is the stack working as designed rather than a gap in it. This PR is the id plumbing: o.bind learns opts.id here, and dynamic_bindings grows the fourth field here. Without both, parse_binding_records sees an empty id and falls back to the description, which is the behaviour the sort commit exists to replace. The sort commit is stacked on this one in my branch, which is why this PR is the one open and the sort PR is not yet. Your three-layer tree is the combination, and the order is load-bearing.

I rebased both commits onto today's quattro (8b4eae66) before measuring any of the below. The patches are byte-for-byte what they were, and every number came out the same as it did on e332dc97.

One number I read differently. Per tree, both files, no failures anywhere:

tree keybindings-menu-test.sh hyprland-bind-id-test.sh
8b4eae66 16 not present
+ #13276 16 not present
+ #13276 + #12557 16 9
+ #13276 + #12557 + sort 20 9

So 29 assertions on the full stack. The 4 the sort commit adds to the first file are that a translated pair still shares a row, that a translated label keeps its sort position, that a translated media key stays in the tail, and that a cached record still carries no id. If your tree counts 10, it may be short one of the two files.

What your config measures

I ran it through the real menu with Hyprland stubbed the way keybindings-menu-test.sh stubs it: every bind reported with its translated description and dispatcher: __lua, ids recovered from the Lua scan of the same config. To have something to compare against, I generated an English twin of your config by putting each bind's id in place of its description, rendered both, and read each render back as a sequence of ids. Each tree is the same base with one more cherry-pick, so no layer is credited with another's effect.

tree rows, English rows, Russian rows holding the same id in the same position
8b4eae66 256 257 7 of 257 (2.7%)
+ #13276 253 257 0
+ #13276 + #12557 253 257 0
+ #13276 + #12557 + sort 253 253 219 of 253 (86.6%)

Anything below 3% there is alignment noise rather than order: with the row counts differing, the two sequences shift apart at the top and never come back. The reading is that on any tree without the sort commit a translated config has essentially nothing in common with the untranslated order, and your config shows that at full size. The menu opens on "Бинды Tmux" where it should open on "Горячие клавиши".

Worth separating out: #13276 is what lets the four paired actions merge at all, which is the English column going from 256 rows to 253. The sort commit is what makes the same thing happen for a translated config.

Two things your config found

The generator drops the id on the two mouse binds. Lines 221 and 223 pass it as a fifth argument:

o.bind("SUPER + mouse:272", "Переместить окно", hl.dsp.window.drag(), { mouse = true }, { id = "Move window" })

o.bind(keys, description, dispatcher, options) takes four, so the table holding the id is dropped and opts.id falls back to the Russian description. Omarchy's own two are at default/hypr/bindings/tiling.lua:73-74 with a single options table, and merging yours the same way is the fix:

o.bind("SUPER + mouse:272", "Переместить окно", hl.dsp.window.drag(), { mouse = true, id = "Move window" })

Those two binds are the entire residual in the table above. With them merged the same render reads 252 of 253 (99.6%), and the two rows move from 117 and 118 to 84 and 85, where the English render has them. The row left over is my comparison being lossy rather than a row out of place: "Предыдущий стол" is the label for both Former workspace and Previous workspace, so reading a Russian label back to a single id cannot pick between them.

A merged row can name the same chord twice. Where an action is bound on both the Latin symbol and the keycode, the menu resolves code:25 through the default keymap rather than the active one, so both halves of the pair print alike:

SUPER + W / SUPER + W               → Закрыть окно

Four rows do this, the four in alternative_chord_ids. On the trees without the sort commit they were two identical rows instead, so this is the pairing fix landing on a pair whose halves render the same, not a new break. Past those four, your config leaves 48 rows byte-identical to another row on every tree I measured (205 distinct rows out of 253, and 205 out of 257 before), which is the dual-bind pattern showing through and is not something either PR touches.

I would rather not fold a chord collapse into this PR while it is under review. Worth a follow-up? The shape would be that a row never prints one chord twice, and that two rows agreeing on rendered chord, id and dispatch become one, which would take those 48 with it.

All of the above is a stubbed Hyprland, so it is the strings the menu builds and not a live session. Your render is still the step that confirms it on screen.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Live-stack report as promised. Tree: PR #12557 base + #13276 fix + id-sort commit (clean applies). Suite: 10/10 green.

Live render on our real RU config (257 generated binds, stock + user layers), stack vs stock script, same session:

  • stock: 302 rows; stack: 298 rows. The 4 fixed rows are exactly translated symbolic+code:NN pairs merging: Close window (SUPER+W + code:25 → one row), scratchpad ×2, Calculator.
  • Translated pairs inside the user config merge; tiers hold (translated rows keep their place, no fall into default 50 observed).
  • Unchanged (pre-existing, out of scope): stock-file vs user-file identical chords still double-list (e.g. SUPER+K twice) — the scan reads both files and stock declares no id, so they never shared a key before either. No regression: those rows are byte-identical in both renders.

So the stack does what was asked on a real 254+ description config. Thanks for the branch — and for catching the scan gap before I bisected it.

@WokoFlipper

Copy link
Copy Markdown

Found it — and it was our plumbing, not the words. o.bind takes exactly 4 args; stock mouse binds already carry a 4th-arg table ({mouse=true}), and our generator appended {id=...} as a 5th, which Hyprland silently drops along with the id. So the two mouse binds fell back to the Russian description as key and never paired — your 252/253.

Fixed on our side (merge into the one table: {mouse=true, id=...}), generator patched the same way, no 5-arg emits left. The gist config is updated: https://gist.github.com/WokoFlipper/7b46567bfb624c61ba928fe2572234b2

Live stack render now shows both mouse rows merged (298 rows total, down from the split state). The remaining dups and the stock-vs-user double listing are pre-existing as you said — dual-bind pattern plus stock declaring no id — out of scope here. Thanks for the table that pointed at exactly the right two rows.

@linyiru

linyiru commented Sep 30, 2026

Copy link
Copy Markdown
Author

Your fix is confirmed here, and more directly than I expected: the config you just published is byte-for-byte the one I had already rendered, so 252 of 253 is now a measurement of your tree rather than a prediction about it. Both mouse binds land where the untranslated reference puts them.

Below, roughly in order of how much it changes what you do: two corrections, then two small notes and a question. One of the corrections is to my own comment above, which crossed yours in the post.

What the fix actually moved

Nothing merged, and no rows went away. On my render of your updated config, before and after:

rows merged rows duplicate row groups
your original 253 4 48
your fix 253 4 48

What changed is where two rows sit, and what they sit next to. Before, in your original config:

114:SUPER + Home                → Вернуть ширину окна
115:SUPER + L                   → Раскладка окон
116:SUPER + L                   → Раскладка окон
117:SUPER + LEFT MOUSE BUTTON   → Переместить окно
118:SUPER + RIGHT MOUSE BUTTON  → Изменить размер окна
119:SUPER + SLASH               → Масштаб монитора +

After, with your fix:

 82:SUPER + RIGHT              → Фокус на правое окно
 83:SUPER + UP                 → Фокус на верхнее окно
 84:SUPER + LEFT MOUSE BUTTON  → Переместить окно
 85:SUPER + RIGHT MOUSE BUTTON → Изменить размер окна
 86:SUPER + MINUS              → Большое окно влево

Those two rows cannot pair with anything. Move window and Resize window are two different actions with one bind each, and neither id is in the four-name allow-list that pairing is gated on. The id was doing its other job here, as the sort key. prioritize_entries gives Move window$ tier 34 and Resize window tier 35, between Focus at 33 and Expand window at 36, which is exactly where the fixed render puts them: 84 and 85, the same two positions and the same neighbours as the untranslated reference. Before the fix the id fell back to the Russian description, which matches no tier pattern, so both rows dropped to the default tier 50 and the secondary sort placed them by their rendered chord, which is why they landed alphabetically between SUPER + L and SUPER + SLASH rather than anywhere meaningful.

Worth separating out because your 298 was already 298 before the fix, so a row count holding still is the expected result here and not a sign the fix did not take.

The rows still double-listing are not a missing stock id

Stock binds do carry an id. o.bind sets opts.id = opts.id or description at default/hypr/helpers.lua:149, so a stock bind's id is its English description. What decides whether two rows merge is id in shares_a_row at bin/omarchy-menu-keybindings:448, tested against the four names at line 372, and any other id never merges no matter how well both halves agree.

SUPER + K is also not stock against user. Your generator emits both halves itself:

hl.unbind("SUPER + K")
o.bind("SUPER + K", "Горячие клавиши", "omarchy-menu-keybindings", { id = "Keybindings" })
hl.unbind("SUPER + code:45")
o.bind("SUPER + code:45", "Горячие клавиши", "omarchy-menu-keybindings", { id = "Keybindings" })

code:45 resolves to K through the default keymap, so both rows print SUPER + K, both already carry id Keybindings, and Keybindings is not one of the four. Stock's own SUPER + K is gone, unbound by your line 280.

Counted with the keymap the menu resolves through: your config declares 52 chord-and-label twins of that shape, all 52 carrying a single id. Four have an id in the allow-list and merge. The other 48 stay as two byte-identical rows, and because all 48 already agree on their id, a collapse keyed on rendered chord, id and dispatch would take every one of them. Out of scope for this PR, agreed, but reachable, and not blocked on ids.

My base row was not a clean control

To get an untranslated reference I built an English twin of your config by replacing each description with its declared id. On the base tree that twin gets help the translated config cannot get: base keys its dispatcher cache on modmask,description,key, and the twin's descriptions are English ids that mostly coincide with stock's own descriptions, so it collects entries the Lua scan wrote for stock binds before it died at qconsole.lua. Measured on the CSV parse_binding_records actually receives, base resolves a dispatcher for 192 of 257 records with the twin and 2 of 257 with your config, and those two are the hardcoded static_bindings rows.

So the 2.7% I published for base is translation plus that artefact rather than translation alone, and the one merge the base English column showed, SUPER CTRL + Q / XF86Calculator, is the artefact and not something your config loses. I also wrote that #13276 "is what lets the four paired actions merge at all, which is the English column going from 256 rows to 253"; the honest version is that the full stack merges four and base merges zero for a translated config, and that 256 was carrying the artefact.

The rows that matter are unaffected. On the full stack your config resolves 250 of 257 dispatchers both before and after your fix, the English twin resolves the same 250, the three agree record for record in id, dispatcher and arg, and all three render 253 rows. That is why 252 of 253 above is a clean number and the 2.7% above it was not.

Two small ones, still true of the updated file

Line 275 binds SUPER + SHIFT + code:201 to Omarchy menu, and that row never reaches the menu: dynamic_bindings drops code:201 on purpose, because the Copilot key duplicates an existing bind. Omarchy menu shows one chord where your file declares two. And the block header reads keys=257 where the file has 256 o.bind calls.

One thing your rig can see and mine cannot

All four merges print their chord twice on my render:

SUPER + W / SUPER + W               → Закрыть окно
SUPER + S / SUPER + S               → Скрытый стол
SUPER ALT + S / SUPER ALT + S       → Окно в скрытый стол
SUPER CTRL + Q / SUPER CTRL + Q     → Калькулятор

You described the first as "SUPER+W + code:25 → one row" without saying what the row reads. If it reads that way live then every merge this stack wins on your config is cosmetically doubled, which is worth knowing before anyone calls the pairing finished. The collapse above fixes it, and I still think it belongs in a follow-up rather than in this PR.

Last thing, unresolved from before: what command gives you 10 tests? Both files together are 29 for me on this stack, 20 in keybindings-menu-test.sh and 9 in hyprland-bind-id-test.sh, no failures in either.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Three answers:

  1. Doubled chords — confirmed live, same as yours: SUPER+W / SUPER+W, SUPER+S / SUPER+S, SUPER CTRL+Q / SUPER CTRL+Q, plus XF86Calculator → Калькулятор on its own row 283. So yes, every merge the stack wins here prints doubled. Agreed it belongs in a follow-up, and the collapse key you sketched (rendered chord + id + dispatch) covers all 48.

  2. Test count — my "10/10" was the tail I quoted, sloppy of me. Full files: 20 + 9 = 29, zero failures. Same as yours.

  3. keys=257 — the header lies, and not by your one: the live block holds 252 o.bind calls. The counter is written by our generator at apply time and the block has since been hand-edited (the mouse merge), so they drifted. Real number is 252; I will resync the header on the next generator run rather than hand-patch it.

On stock ids: noted — opts.id defaults to description (helpers.lua:149), so the double listing is purely the four-name allow-list gating, not a missing id. That also confirms the 48 stay split by design here, collapse or not.

@WokoFlipper

Copy link
Copy Markdown

Pinging @omacom/core-team for review — this one is ready from the outside:

  • Technically clean (mergeable, no conflicts), tests green (29/29 across both files).
  • Validated live on a real 257-bind translated config: pairing + tiers hold, 302→298 rows with exactly the translated pairs merging.
  • Works as designed only together with fix(keybindings): keep the Lua bind scan alive past qconsole.lua #13276 (scan fix) + the id-sort follow-up; each piece alone is nearly a no-op — worth reviewing as a stack.
  • No CI runs on the branch, in case checks are expected before merge.

Happy to rebase, split, or measure anything missing.

@linyiru

linyiru commented Sep 30, 2026 •

Copy link
Copy Markdown
Author

Thanks for the ping, and for doing the live validation that made it worth pinging about. Two of your four bullets I can confirm independently, one number is mine to correct before a reviewer trips over it, and the third piece of the stack is mine to fix.

Mergeable, and no checks: both confirmed. MERGEABLE / CLEAN against quattro right now, and gh pr checks reports no checks on the branch, so nothing is pending or failing out of sight. One thing a maintainer may want to know: the head 647b4a21 is based on e332dc97, and I have both commits rebased onto today's 8b4eae66 locally with unchanged patch ids. I deliberately have not force-pushed, because GitHub still reports the merge clean and 647b4a21 is the exact commit you tested by SHA. Say the word and the rebase goes up.

The test count is the stack's, not this PR's. Measured just now on four trees, each one cherry-pick apart, no failures anywhere:

tree keybindings-menu-test.sh hyprland-bind-id-test.sh total
quattro 16 absent 16
+ this PR 16 9 25
+ #13276 + this PR 16 9 25
+ #13276 + this PR + sort 20 9 29

So a reviewer running both files on this branch should expect 25. The four extra assertions belong to the sort commit, which is not in this PR: that a translated pair still shares a row, that a translated label keeps its sort position, that a translated media key stays in the tail, and that a cached record still carries no id.

The sort commit had no PR, which was a real gap in what you just asked for, so it is open now: #13888. I had been holding it back because opening it against quattro while this one is unmerged shows two commits in a single diff. It does, and the body says so: only the second commit belongs to that PR, the first is this one. Reviewing the stack as two PRs plus #13276 is easier than reviewing one PR plus a branch you have to go find, which is what you were left with.

One precision on the merging bullet, since it is the part a reviewer is most likely to read as a guarantee: the four rows that merge are the four actions in alternative_chord_ids, not translated pairs in general. Your config declares 52 chord-and-label twins, 4 of them have an allow-listed id and merge, and the other 48 stay as two identical rows on every tree I measured. That is the dual-bind pattern showing through rather than anything either PR regresses, and the accounting is in my comment above. Collapsing those 48 is reachable and I would rather offer it as a follow-up than widen this PR under review.

Last thing, for whoever picks this up, and the credit belongs where it is due: everything I have measured is a stubbed Hyprland, so it is the strings the menu builds. The live confirmation in this thread is @WokoFlipper's, on their own 257-bind translated config, and it is the part I could not produce myself. The scan fix the whole stack rests on is @playGitboy's #13276, without which none of this reaches a user's own binds. @llstrk flagged the regression that #13276 was missing, and @manuaudio is who pointed me at it when I filed a duplicate.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Word said: yes, please rebase onto today's quattro — a fresh base can only help reviewers, and our live validation re-runs in minutes if you want it re-confirmed post-rebase.

Corrections accepted: 25 on this PR alone (16+9), 29 with the sort commit — our 29 was the full stack, should have labeled it so. Same for the merging precision: 4 allow-listed ids, not translated pairs in general. Will cite it that way going forward.

And thanks for opening #13888 — reviewing two PRs plus #13276 beats hunting a branch. The stack is now in the shape you described: will re-run the live render once the rebased base lands, if useful.

o.bind wrote the id fallback into the table the caller handed it, and that
fallback only fills an id that is not set yet, so a config reusing one options
table across several binds gave every later bind the first one's id. The
description hid it by being assigned outright on every call.
@linyiru

linyiru commented Sep 30, 2026

Copy link
Copy Markdown
Author

greptile reviewed the stacked PR (#13888) and one of its three findings is in this branch rather than that one, so the fix is here: o.bind wrote the id fallback into the options table the caller handed it.

local opts = options or {} takes the caller's table, and opts.id = opts.id or description only fills an id that is not set yet. So a config handing one options table to several binds gave every bind after the first the first one's id. The description never showed it, because that one is assigned outright on every call. Three binds sharing a table all reported id: First bind.

Nothing in the repo reaches it: the only local opts is inside o.bind itself, and no call site passes a variable as the fourth argument. A user config can, and o.rebind hands its options straight through, so it does not need anything in-tree to change first. Worth recording that the fix that looks obvious is not one: computing the id into a local and then writing opts.id = id leaks in exactly the same way. The table has to be copied, which is what the new commit does.

test/shell.d/hyprland-bind-id-test.sh goes 9 to 10, and the assertion added is the only one in the file that fails when the copy is taken back out. So the count I gave in the comment above moves by one:

tree keybindings-menu-test.sh hyprland-bind-id-test.sh total
quattro 16 absent 16
+ this PR 16 10 26
+ #13276 + this PR + sort 22 10 32

Two things about the push, since both change what you would be looking at. Adding the commit rewrites the branch, so 647b4a21 is no longer the head: the rebase onto today's quattro that I had been holding back goes up with it, because the reason for holding it was that the merge was clean and the SHA you tested was still on the tip. Neither is true once the branch moves anyway. @WokoFlipper, this means the commit you validated by SHA is superseded even though its patch id has not changed. Nothing about the id behaviour you measured moves; the branch is one commit longer and rebased.

Thanks to greptile for the finding, and to @WokoFlipper for the live run that got the stack looked at closely enough for it to surface. The scan fix the whole stack still rests on is @playGitboy's #13276.

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Keybinding system now uses stable identifiers.

The PR appears safe to merge; no new blocking issue was established.

Findings

  1. P2 Test skips the real config ▶

Summary

The PR adds stable ids to Lua binds, uses those ids to recover menu dispatchers, and explicitly identifies web-app focus binds. The latest changes align tab and newline handling between the Lua scan and menu parser and invalidate cached records.

  • Adds tests for the normalized-description lookup through a custom config.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["Lua bind declaration"] --> B["Scan: description, key, id, command"]
  C["Hyprland bind report"] --> D["Normalize reported description"]
  B --> E["Resolve key and id"]
  D --> E
  E --> F["Look up dispatcher by id and key"]
  F --> G["Menu record"]
Loading

Reviews (3) · Last reviewed commit: "Look a bind's label up the way Hyprland ..."

Comment thread default/hypr/helpers.lua
Comment thread bin/omarchy-menu-keybindings
Comment on lines +87 to +94
cat >"$home/.config/hypr/hyprland.lua" <<LUA
dofile("$ROOT/default/hypr/bootstrap.lua")
require("default.hypr.helpers")
require("default.hypr.bindings.applications")

o.bind("SUPER + ALT + Z", "Renamed later", { webapp = "https://example.test/", focus = true }, { id = "Stable Id" })
hl.bind("SUPER + ALT + Y", hl.dsp.exec_cmd("true"), { description = "Written against hl" })
LUA

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test skips the real config This test loads a shortened config before adding its custom-id binds. It can pass even if loading the real config stops before personal bindings are reached, leaving the menu’s recovery of those binds’ ids and commands untested. Exercise that path through the real config, stubbing dependencies as needed.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@linyiru

linyiru commented Sep 30, 2026

Copy link
Copy Markdown
Author

Correcting the scope of my report above. I wrote that the generator drops the id on the two mouse binds, and those were only the two I happened to look at. The current gist has 29 more with the same five-argument shape, all XF86 keys, and every one still loses its declared id.

o.bind takes four arguments. o.bind("XF86AudioMute", "Выключить звук", "omarchy-audio-output-volume mute-toggle", { locked = true }, { id = "Mute" }) passes five, so Lua discards the id table and the id falls back to the Russian description.

Measured through the real o.bind and written up on #13888 rather than repeated here: #13888 (comment). Nothing in the menu reads those ids today, because prioritize_entries classifies an XF86 chord by the chord and sorts within that tier on the display column, so no number on either PR moves. @WokoFlipper, the one-line table merge you already applied to the mouse binds is the same fix for these 29.

🤖 Generated with Claude Code

Copying the options table keeps the id fallback out of the caller's own
table, but a copy made key by key sees only what that table holds
itself. pairs walks no further, so a config that keeps its shared flags
on one table and points each bind's options at it through __index used
to hand Hyprland those flags and now hands it a copy without them.

The copy carries the metatable across, so a lookup on it still finds
what the original inherited. Assigning into the copy still leaves the
caller's table alone, which is what the copy was for.

greptile's review on omacom#13923 pointed this out.
The four web app binds that focus an open window match it against their
id, so each has to name one rather than inherit a description a
translator may move. The assertion for that read the binding files a
line at a time, which only holds while every one of those calls is
written on a single line: take the id off one and spread the call over
four lines and the grep stops seeing the bind at all, counts three of
the four, and reports them all well formed.

o.bind knows which binds those are however they are written, and it is
also the only place the distinction is visible: by the time a record
reaches the menu, an id a bind named itself and one that defaulted to
the description are the same string. The check now wraps o.bind, loads
every binding file Omarchy ships, and asks for each web app bind that
focuses a window whether its caller passed an id.

greptile's review on omacom#13923 pointed out the line-at-a-time hole.
The menu's Lua scan reports one bind per line as tab separated fields,
three of which are the config author's own text: the description, the id
this branch adds, and the command. A tab in any of them moves every
field after it one column along, and a newline splits the bind into two
records. With a tab in a description the reader on the other side takes
the second half for the key and the key for the dispatcher, which is a
wrong row today and a wrong cache key once rows are keyed on the id.

Each field is folded to one line before it is printed. A bind is worth
reporting with its label squeezed and not worth losing.

greptile's review on omacom#13923 found this on the id field. The description
has carried it as long as the scan has, so the fix covers every field
rather than the one the review landed on.
@linyiru

linyiru commented Oct 1, 2026

Copy link
Copy Markdown
Author

Three commits added, all of them answers to a review greptile left on the validation PR, #13923. That PR stacks on this branch, so its diff carries these two commits and the review landed on them there. Nothing about what this PR is for changes.

639d1260 keeps the flags a bind inherits. Copying the options table is what stops the id fallback from leaking into the next bind, but a copy made key by key sees only the keys a table holds itself, so a config that keeps shared flags on one table and points each bind's options at it through __index used to hand Hyprland those flags and would have handed it a copy without them. The copy carries the metatable across now. Measured on a table with locked behind __index: nil after the copy, true with the metatable, and the caller's own table still untouched.

2693c9f2 asks o.bind which web app binds focus a window instead of grepping the binding files for them. The old check read a line at a time, so taking the id off one of those four binds and spreading the call over four lines left the grep finding three binds and reporting all three well formed. Reading the loaded bind records cannot replace it either: all four declare an id equal to their description, so a record keeps no trace of which of the two it came from. o.bind is where that difference exists, and it does not care how the call is laid out.

2709156b keeps a bind's own text out of the scan's delimiters. The menu's Lua scan reports one bind per line as tab separated fields, and three of them are the config author's text. On quattro today a bind described as Before<tab>After prints six fields where the reader expects five, so the reader takes After for the key and K for the dispatcher; a newline prints two records for one bind. This branch adds a second field of that kind and keys the cache on it, so every field is folded to one line now rather than only the new one.

No config any of us has measured on this thread is affected by the first of those: there is no setmetatable in Omarchy's own bindings, nor in any of the three revisions of the generated config in @WokoFlipper's gist. The other two are a test and the scan's record shape.

On one Arch box, ./test/shell goes from 3961 ok / 7 not ok at 125284e2 to 3963 / 7 here, the two added assertions and nothing else. ./test/cli is 88 / 1 on both. The same seven and the same one fail on that box before any of this.

Thanks to @WokoFlipper, whose generated config is what sent me looking for the validation in the first place, and to greptile for reading the tests as closely as the code.

🤖 Generated with Claude Code

Comment thread bin/omarchy-menu-keybindings Outdated
@WokoFlipper

Copy link
Copy Markdown

Checked our side against all three: no setmetatable in the live config, no tabs or newlines in catalog keys/values or the bindings table — so the metatable and delimiter fixes change nothing for us, as you predicted. The validation PR is the right home for them.

Thanks for the detailed accounting (and for the credit). Standing by for whatever the reviewers need measured next.

The scan folds a tab in a description to a space so the record it prints keeps
its fields, but the menu looked the bind up by the description `hyprctl binds`
reported, tab and all, so the two keys could never meet: the bind came back
with no id and no dispatcher. The reported description reaches the tab
delimited record too, where the menu reads the back half of a label as the
dispatcher it would run -- a bind labelled "Tabbed\tkillactive" dispatches
killactive when it is picked, on quattro as much as here.

So fold the delimiters out of the reported description the same way, and cut
the scan's copy at the first newline, which is where hyprctl's own output cuts
the description it reports. Bump the cache version, or an install that already
cached a broken record keeps serving it. The literal skips to v17 because v15
is taken by omacom#7564 and omacom#8229 and v16 by omacom#13888, all open: two branches setting
that line to the same value merge with no conflict and leave no bump at all.

A comma in a label still shifts the fields of the intermediate record, here and
on quattro alike. That one is the record format itself, not this lookup.
@linyiru

linyiru commented Oct 1, 2026

Copy link
Copy Markdown
Author

greptile is right, and the half it did not say out loud is the sharp one: the reported description reaches the record too, and the record is tab delimited, so the menu reads the back half of a label as the dispatcher it would run. Measured with a bind labelled Tabbed<TAB>killactive, one bind in the config, picking the row from the menu:

tree record what picking it ran
8b4eae66 (quattro, this branch's base) 4 fields, → Tabbed, field 2 killactive hyprctl dispatch killactive
2709156b 4 fields, → Tabbed, field 2 label hyprctl dispatch label
d3c51823 3 fields, → Tabbed killactive, field 2 exec the command the bind declared

So a label with a tab in it dispatched its own second word before this branch existed, and folding the scan's copy fixed the record the scan prints without fixing the one the menu builds. A newline is the same thing one step on, and that half is this branch's own: 2709156b folded it to a space, hyprctl binds prints one field to a line and cuts the description there instead, and the two could never meet, so a bind labelled across two lines came back with no id and no dispatcher at all.

The fix

d3c51823 folds a tab out of the reported description the same way the scan folds its own copy, and cuts the scan's copy at the first newline, which is where hyprctl's output cuts the one it reports. The description is the one field the two sides match on, so the rule is that the scan stores it as hyprctl would report it, not as the config wrote it.

It also bumps the record cache, since without that an install that already cached a broken record keeps serving it. #13521 bumped v13 to v14 for the same reason. The literal goes to v17 rather than v15: #7564 and #8229 both take v15 and both are still open, and #13888 takes v16, so this is the rule from that PR applied again. Two branches setting that line to the same value merge with no conflict and leave no bump at all, which is the failure the bump is there to avoid. If this lands before #13888, that PR's bump has to be raised again, and that is a conflict I would rather have than not see.

The test

greptile's second point was the one worth having: the old assertions read the scan's six fields and never ran the menu. The new one goes through the menu and reads the record it cached, three binds in a config of its own, and asserts each resolves exec true: a plain label as the control, a label carrying a tab, and a label Hyprland cut short at a newline. On the same box and the same day, ./test/shell goes from 3963 ok / 7 not ok at 2709156b to 3964 / 7 here, and ./test/cli is 88 / 1 on both. The seven and the one fail on that box before any of this.

What is left, stated rather than implied

A comma in a label still shifts the fields, because the record between dynamic_bindings and parse_binding_records is comma separated: Comma, label renders as → Comma with label in the dispatcher column, identical on 8b4eae66 and here. That is the record format rather than this lookup, and a real fix means the record not being comma separated at all, which static_bindings currently relies on. I offered it as a separate PR on #13888 (#13888 (comment)) and the offer stands, here or there, whichever a maintainer would rather read.

And @WokoFlipper's check

Thank you for putting a live config against three commits the same day they landed. The half I can see agrees: gist rev4 has no setmetatable, getmetatable or __index in it, no literal tab, and no \t or \n inside a quoted string, across all 262 bind call sites. One precision for anyone reading the stack: all of these commits sit on this branch. #13923 shows them because it stacks here, not because they moved; what lives there is the warning that found the problem, not the fix.

Worth saying plainly rather than in passing, since it is most of why this branch is measured instead of argued: ten comments on this PR since 22 September and four more on #13888, a 262-bind config published and refreshed four times, a stack built and run on a live Cyrillic layout, your own generator's plumbing bug found and fixed by you mid-thread, and the doubled rows called out rather than left for me to meet in a screenshot. Every number in this PR that is about a real config instead of a stub came from your box. Thank you for the time that took, @WokoFlipper.

Still on 8b4eae66 and still merging clean, so the rebase you asked for on 30 September is in. Say if you want it moved to today's quattro as well and I will do that rather than let it drift.

If you want something to measure next, the one thing only a live session can settle is on #13888: moving dedupe_binding_records after parse_keycodes, which on rev4 takes 259 rows to 211 and turns your four doubled rows into one real pair. Numbers are at #13888 (comment), and whether it belongs in that PR is still your call.

Thanks to greptile for the find. A P2 that turns out to dispatch a word out of the user's own label is worth more than the label it came from.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Thank you — and back at you: the tab-dispatch table is genuinely scary (a label running its own second word), glad d3c5182 closes it on both sides. The v17 skip-ahead rule is noted, will watch for collisions the same way.

Our box stays available for the stack: live Cyrillic config, same-day runs, gist refreshes on request. And noted on the comma record format — agree it wants its own PR, whenever a maintainer picks a side.

linyiru added a commit to linyiru/omarchy that referenced this pull request Oct 1, 2026
The records are sorted before they are cached, and the cache key hashes the
version literal, the active keymap and `hyprctl binds` -- none of which moves
when this script's sort does. So an install that already has a cache keeps
serving the order it was written with, and since en_US.UTF-8 is the default
locale that is most installs: the pinned order would arrive later, at some
unrelated moment when a bind or the keymap changed. Measured over one cache
directory and the two chords that swap, the new script printed the old
script's en_US order byte for byte.

Bump the version so that cache is retired, and plant a record under the key
the previous version would have written, so that changing the order without
a bump fails here rather than in a user's menu.

The literal skips to v18 because v15 is taken by omacom#7564 and omacom#8229, v16 by
omacom#13888 and v17 by omacom#12557, all open: two branches setting that line to the
same value merge with no conflict and leave no bump at all. omacom#13521 bumped
v13 to v14 for a records change the same way.
@linyiru

linyiru commented Oct 1, 2026

Copy link
Copy Markdown
Author

Correcting one clause in my comment above. I wrote that #13923 shows these commits because it stacks here. It does not, and it is wrong in the direction that matters.

#13923's commit list is 9b06086a, 125284e2, 1c47726e, 64ca9f06, c9d72322. That branch left this one at 125284e2 on 30 September and the three fixes went on here the following morning, so the only commits of mine it shows are the two that predate them. The clause half a sentence later, that what lives there is the warning rather than the fix, is the accurate half of the same thought.

What that costs, measured rather than reasoned

greptile's P1 was about the id field, so here it is on that field rather than on the description it shares an emit line with. One bind, o.bind("SUPER + ALT + X", "Plain label", "true", { id = "Tabbed<TAB>killactive" }), picked from the menu:

tree the record the menu held what picking the row ran
125284e2, where #13923 branched → Plain label, dispatcher empty nothing
c9d72322, #13923 as published → Plain label, dispatcher empty nothing
d3c51823, here → Plain label, exec, true the command the bind declared

So greptile had it exactly: unusable from the menu, and silently, since the row still renders and still looks fine. The tabbed description from the table in my earlier comment behaves the same way at c9d72322 as at 125284e2: 4 fields, and picking it dispatches killactive out of the label's own second word.

Both of those are still live on the head of the PR they were reported against. I said as much on #13923 at the time, that the other three are fixed on the branch they stack on, so that thread is not misleading. This one was.

The ordering that falls out

#13923 is based on quattro and carries 9b06086a, so merging it on its own puts opts.id into the record with none of the folding: one_line appears 0 times at c9d72322 and 7 times here. quattro has neither field nor fold today, so that would not introduce the tab defect, it would widen it by one more field that a config author writes. This PR goes first, or #13923 gets rebased before it lands.

The rebase, re-proved against the head as published

I also wrote on #13923 that the branches above this one rebase onto the new head "with no conflict, tested". d3c51823 was hours old when I wrote that, so here it is against the head as published, nothing pushed:

git rebase --onto d3c51823 125284e2 replays all three warning commits clean. On the rebased tree both binds above resolve the command they declared, and the three test files this stack owns read hyprland-bind-warnings-test.sh 10 ok / 0 not ok, hyprland-bind-id-test.sh 12 / 0, keybindings-menu-test.sh 17 / 0, against 10 / 0, 10 / 0 and 16 / 0 at c9d72322. The two deltas are this branch's own new assertions arriving, not the warnings changing.

I am leaving #13923's head where greptile reviewed it and where @WokoFlipper measured rev4 at 0 arity warnings and 0 stray ids, for the same reason this branch has not moved either: it is MERGEABLE, and a force-push rewrites a head somebody tested by sha for no mechanical gain. Say so and I will push the rebase instead of waiting for this to land.

And @WokoFlipper

Thank you, and the standing offer matters more here than it sounds: every defect this stack has turned up so far came out of text a config author wrote rather than out of code, and a stub cannot be surprising on purpose. I will take you up on it for the dedupe placement once that call is yours to make. The comma record format can wait for a maintainer to pick a side, and it costs nothing sitting where it is.

Thanks again to greptile for the P1. It read as a field-shifting bug and it is worse than that: the field it shifts is the one the lookup keys on, so the bind does not misfire, it stops existing.

🤖 Generated with Claude Code

@WokoFlipper

Copy link
Copy Markdown

Noted on all three (commit-list correction, rebase standing by, comma format waiting). The standing offer stands on this side too — live box, same-day runs whenever the dedupe placement (or anything else) needs a real config. Thanks for keeping the accounting honest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants