Repository navigation
Conversation
|
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: 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. 4. qconsole gap. Reproduced on a live user config ( Our side, done: our generator now emits |
|
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 Repro: press SUPER+C (stock Root: the key name resolves through the active keymap, and there is no Workaround, proven live on our box: dispatch with X11 keycodes instead of names — Ask: layout-independent injection in |
e23f6de to
647b4a2
Compare
|
Rebased onto current 1. 2. The ratchet: the count is the wrong thing to pin. Measured on the rebase. 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 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 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 4. qconsole. Mine to file, and thank you for the offer. Reconfirmed on current 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 reportDifferent subsystem from this PR: What I can add from the source, which widens it: four stock binds are in that class, not one. One question before |
|
Correcting something I wrote in the body and repeated to @WokoFlipper above: I said the Seven open PRs already fix some part of that scan: #7564, #8876, #12712, #12916, #13276, #12155 and #11733. Three open issues cover the runaway 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 @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 |
|
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. |
|
@WokoFlipper the follow-up branch exists: git fetch https://github.com/linyiru/omarchy menu-sort-by-bind-id && git checkout FETCH_HEADIt takes the id through 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. So I put the fixture from my own tests where a real user would put it, in The branch is necessary but not sufficient. If your 254 descriptions live in 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 gh pr checkout 13276
git fetch https://github.com/linyiru/omarchy menu-sort-by-bind-id
git merge FETCH_HEADThey 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
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 🤖 Generated with Claude Code |
|
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.
|
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: I rebased both commits onto today's quattro ( One number I read differently. Per tree, both files, no failures anywhere:
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 measuresI ran it through the real menu with Hyprland stubbed the way
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 foundThe 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("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 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 Four rows do this, the four in 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 |
|
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:
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. |
|
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. |
|
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 movedNothing merged, and no rows went away. On my render of your updated config, before and after:
What changed is where two rows sit, and what they sit next to. Before, in your original config: After, with your fix: Those two rows cannot pair with anything. 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 idStock binds do carry an id.
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" })
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 controlTo 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 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, 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 fileLine 275 binds One thing your rig can see and mine cannotAll four merges print their chord twice on my render: 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 🤖 Generated with Claude Code |
|
Three answers:
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. |
|
Pinging @omacom/core-team for review — this one is ready from the outside:
Happy to rebase, split, or measure anything missing. |
|
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. 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:
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 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 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 |
|
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.
647b4a2 to
125284e
Compare
|
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:
Nothing in the repo reaches it: the only
Two things about the push, since both change what you would be looking at. Adding the commit rewrites the branch, so 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 |
|
| 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 |
There was a problem hiding this comment.
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!
|
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.
Measured through the real 🤖 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.
|
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.
No config any of us has measured on this thread is affected by the first of those: there is no On one Arch box, 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 |
|
Checked our side against all three: no 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.
|
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
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: The fix
It also bumps the record cache, since without that an install that already cached a broken record keeps serving it. #13521 bumped The testgreptile'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 What is left, stated rather than impliedA comma in a label still shifts the fields, because the record between And @WokoFlipper's checkThank you for putting a live config against three commits the same day they landed. The half I can see agrees: gist rev4 has no 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 If you want something to measure next, the one thing only a live session can settle is on #13888: moving 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 |
|
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. |
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.
|
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 What that costs, measured rather than reasonedgreptile'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,
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 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 The rebase, re-proved against the head as publishedI also wrote on #13923 that the branches above this one rebase onto the new head "with no conflict, tested".
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 And @WokoFlipperThank 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 |
|
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. |
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.bindgainsopts.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: __luaand reports no id at all. Running that scan over a stock config before and after: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_keytherefore 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:
keybindings-menu-test.shfailed 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 againsthl.bindstill gets one, and the four web app binds in the tree each name their own id so a fifth cannot quietly inherit one../test/cliand./test/shellwere run on the branch and onquattroon 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 companionomarchy-pkgsclone). 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 formcommand_fromalready 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 thepcallaround it swallows that silently, sorequire("hypr.bindings")never runs. Anything in~/.config/hypr/bindings.luareaches the menu without a resolved dispatcher. Identical onquattro, 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 onquattro, 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