diff --git a/bin/omarchy-menu-keybindings b/bin/omarchy-menu-keybindings index 153a4d29432..0ec90f59614 100755 --- a/bin/omarchy-menu-keybindings +++ b/bin/omarchy-menu-keybindings @@ -5,7 +5,13 @@ # Hyprland's Lua config provider currently reports Lua binds as dispatcher # __lua in `hyprctl binds`. Keep a lightweight source-derived cache so the # menu can still show and dispatch those bindings. +# +# What a bind dispatches is looked up by its id, so renaming what the bind is +# called cannot move it. Hyprland's output carries no id, so the first two maps +# below take a reported description and hand back the key and the id it was +# declared with; they are the whole of what still reads display text. declare -A LUA_BIND_KEY_MAP +declare -A LUA_BIND_ID_MAP declare -A LUA_BIND_DISPATCHER_MAP declare -A LUA_BIND_ARG_MAP @@ -69,15 +75,17 @@ parse_keycodes() { # Supplement `hyprctl binds` for Lua-only binds that Hyprland currently # reports as dispatcher __lua and without their original key for code: binds. build_lua_bind_cache() { - local modmask description key dispatcher arg cache_key + local modmask description id key dispatcher arg cache_key omarchy-cmd-present lua || return 0 - while IFS=$'\t' read -r modmask description key dispatcher arg; do - [[ -z $modmask || -z $description || -z $key ]] && continue + while IFS=$'\t' read -r modmask description id key dispatcher arg; do + [[ -z $modmask || -z $description || -z $id || -z $key ]] && continue LUA_BIND_KEY_MAP["$modmask,$description"]="$key" - cache_key="$modmask,$description,$key" + LUA_BIND_ID_MAP["$modmask,$description,$key"]="$id" + + cache_key="$modmask,$id,$key" LUA_BIND_DISPATCHER_MAP["$cache_key"]="$dispatcher" LUA_BIND_ARG_MAP["$cache_key"]="$arg" done < <( @@ -203,6 +211,11 @@ hl = setmetatable({ local modmask, key = split_keys(keys) local kind = "" local arg = "" + -- o.bind fills this in; a bind written against hl.bind directly may not. + local id = opts.id + if id == nil or id == "" then + id = opts.description + end if type(bind_dispatcher) == "table" and bind_dispatcher.__omarchy_dispatcher then kind = bind_dispatcher.kind or "" @@ -215,7 +228,7 @@ hl = setmetatable({ arg = o.bind_commands[bind_dispatcher] end - print(table.concat({ tostring(modmask), opts.description, key, kind, arg }, "\t")) + print(table.concat({ tostring(modmask), opts.description, id, key, kind, arg }, "\t")) end return noop @@ -273,7 +286,7 @@ modmask_to_text() { # - Map numeric modifier key mask to a textual rendition # - Output comma-separated values that the parser can understand dynamic_bindings() { - local modmask key keycode description dispatcher arg modifiers cache_key + local modmask key keycode description dispatcher arg id modifiers cache_key # Parse the plain `hyprctl binds` output rather than `hyprctl -j binds`: # Hyprland 0.56.0 emits invalid JSON for binds (misaligned fields in @@ -297,12 +310,24 @@ dynamic_bindings() { key="code:$keycode" fi + # The loop-generated binds reach here with neither key nor keycode, and + # their description is the only thing Hyprland gives to find the key by. + # That makes this the one lookup that cannot key on the id: recovering an + # id needs the key it is looking for. if [[ -z $key && -n $description ]]; then key="${LUA_BIND_KEY_MAP["$modmask,$description"]}" fi + # With a key in hand, the description resolves to the id the bind was + # declared with. Everything below this line keys on the id. + id="${LUA_BIND_ID_MAP["$modmask,$description,$key"]}" + + # A bind the cache has nothing for still has to come out of here with no + # dispatcher at all: __lua is not something the menu can run, and a row + # that kept it would read as agreeing with every other row that also + # failed to resolve. if [[ $dispatcher == "__lua" && -n $description && -n $key ]]; then - cache_key="$modmask,$description,$key" + cache_key="$modmask,$id,$key" dispatcher="${LUA_BIND_DISPATCHER_MAP["$cache_key"]}" arg="${LUA_BIND_ARG_MAP["$cache_key"]}" fi diff --git a/default/hypr/bindings/applications.lua b/default/hypr/bindings/applications.lua index e4428a9fb38..e852d3ebeb6 100644 --- a/default/hypr/bindings/applications.lua +++ b/default/hypr/bindings/applications.lua @@ -25,10 +25,14 @@ if o.preinstalled_bindings_enabled() then o.bind("SUPER + SHIFT + E", "Email", { webapp = "https://app.hey.com" }) o.bind("SUPER + SHIFT + ALT + E", "New email", { webapp = "https://app.hey.com/messages/new?display=standalone&new_window=true" }) o.bind("SUPER + SHIFT + Y", "YouTube", { webapp = "https://youtube.com/" }) - o.bind("SUPER + SHIFT + ALT + G", "WhatsApp", { webapp = "https://web.whatsapp.com/", focus = true }) - o.bind( "SUPER + SHIFT + CTRL + G", "Google Messages", { webapp = "https://messages.google.com/web/conversations", focus = true }) - o.bind("SUPER + SHIFT + P", "Google Photos", { webapp = "https://photos.google.com/", focus = true }) - o.bind("SUPER + SHIFT + S", "Google Maps", { webapp = "https://maps.google.com/", focus = true }) + -- These four focus an open window instead of opening a second one, by + -- matching their id against the window's own class or title. That makes the + -- id a pattern the web app decides, not a label of ours, so it is spelled out + -- rather than inherited from the description next to it. + o.bind("SUPER + SHIFT + ALT + G", "WhatsApp", { webapp = "https://web.whatsapp.com/", focus = true }, { id = "WhatsApp" }) + o.bind( "SUPER + SHIFT + CTRL + G", "Google Messages", { webapp = "https://messages.google.com/web/conversations", focus = true }, { id = "Google Messages" }) + o.bind("SUPER + SHIFT + P", "Google Photos", { webapp = "https://photos.google.com/", focus = true }, { id = "Google Photos" }) + o.bind("SUPER + SHIFT + S", "Google Maps", { webapp = "https://maps.google.com/", focus = true }, { id = "Google Maps" }) o.bind("SUPER + SHIFT + X", "X", { webapp = "https://x.com/" }) o.bind("SUPER + SHIFT + ALT + X", "X Post", { webapp = "https://x.com/compose/post" }) end diff --git a/default/hypr/helpers.lua b/default/hypr/helpers.lua index 05f8f91e51b..563894522ba 100644 --- a/default/hypr/helpers.lua +++ b/default/hypr/helpers.lua @@ -87,7 +87,7 @@ local function shell_dispatcher(kind, target, command) return command end -local function command_from(value, description) +local function command_from(value, id) if type(value) ~= "table" then return value end @@ -112,7 +112,7 @@ local function command_from(value, description) return o.launch(value.launch) elseif value.webapp then if value.focus then - return o.launch_webapp_sole(description, value.webapp) + return o.launch_webapp_sole(id, value.webapp) else return o.launch_webapp(value.webapp) end @@ -135,25 +135,69 @@ function o.preinstalled_bindings_enabled() return not file_exists((os.getenv("HOME") or "") .. "/.local/state/omarchy/preinstalls-removed") end -function o.bind(keys, description, dispatcher, options) - local opts = options or {} +-- A bind written wrong says so on stderr. Not stdout: the keybindings menu +-- loads the whole config under a stub and reads bind records off what hl.bind +-- prints there, so a warning on that stream would arrive as a row. +local function warn(message) + io.stderr:write("omarchy: " .. message .. "\n") +end + +function o.bind(keys, description, dispatcher, options, ...) + -- Lua discards an argument a function does not declare, so a bind written with + -- a fifth one still binds and still works, having quietly lost whatever was in + -- it. An id passed that way is the case that bites: the bind falls back to its + -- description, and the only symptom is a row somewhere else in the menu. + local extra = select("#", ...) + + if extra > 0 then + warn("the bind for " .. tostring(keys) .. " passes " .. (4 + extra) .. + " arguments and Lua drops everything past the fourth. An id belongs in the " .. + "options table: o.bind(keys, description, dispatcher, { id = \"...\" }).") + end + + -- Copy what the caller handed over rather than writing into it. The id below + -- fills in only when none is set yet, so a config reusing one options table + -- across several binds would give every later bind the first one's id. + local opts = {} + + for key, value in pairs(options or {}) do + opts[key] = value + end if description then opts.description = description end - dispatcher = command_from(dispatcher, description) + -- A description is what a bind is called; an id is what it is. Anything that + -- has to recognize a bind again later (a cache key, a window match) keys on + -- the id, so a renamed description does not drag behaviour along with it. + -- The description is the only name most binds need, so it is the default. + opts.id = opts.id or description + + local command = command_from(dispatcher, opts.id) + + -- command_from hands back the table it was given when it recognises none of + -- its keys, so anything else means the table was a dispatcher it understood. + -- An id there is read by nothing, and the bind falls back to its description: + -- the same silent loss as a fifth argument, reached from the other direction. + -- Scoped to the tables command_from recognises on purpose, since a table it + -- passes straight through could be anything Hyprland's own API hands back. + if type(dispatcher) == "table" and dispatcher.id ~= nil and command ~= dispatcher then + warn("the bind for " .. tostring(keys) .. " sets id = \"" .. tostring(dispatcher.id) .. + "\" inside its dispatcher table, where nothing reads it. An id belongs in the options " .. + "table: o.bind(keys, description, dispatcher, { id = \"...\" }).") + end - if type(dispatcher) == "string" then - dispatcher = hl.dsp.exec_cmd(dispatcher) + if type(command) == "string" then + command = hl.dsp.exec_cmd(command) end - hl.bind(keys, dispatcher, opts) + hl.bind(keys, command, opts) end -function o.rebind(keys, description, dispatcher, options) +function o.rebind(keys, description, dispatcher, options, ...) hl.unbind(keys) - o.bind(keys, description, dispatcher, options) + o.bind(keys, description, dispatcher, options, ...) end function o.launch(command) @@ -202,8 +246,10 @@ function o.launch_sole(match, command) return "omarchy-launch-or-focus " .. shell_quote(match) .. " " .. shell_quote(o.launch(command)) end -function o.bind_toggle(keys, description, toggle, options) - o.bind(keys, description, "omarchy-toggle-" .. toggle, options) +-- Hands on whatever it was given past its fourth argument, so a toggle written +-- with a fifth one reaches the warning in o.bind rather than losing it here. +function o.bind_toggle(keys, description, toggle, options, ...) + o.bind(keys, description, "omarchy-toggle-" .. toggle, options, ...) end function o.notify(message) diff --git a/test/shell.d/hyprland-bind-id-test.sh b/test/shell.d/hyprland-bind-id-test.sh new file mode 100755 index 00000000000..26a47feb516 --- /dev/null +++ b/test/shell.d/hyprland-bind-id-test.sh @@ -0,0 +1,127 @@ +#!/bin/bash + +source "$(dirname "${BASH_SOURCE[0]}")/base-test.sh" + +require_command lua + +tmpdir=$(mktemp -d) && [[ -n $tmpdir && -d $tmpdir ]] || + fail "the test gets a temporary directory to load the Lua config in" +trap 'rm -rf "$tmpdir"' EXIT + +# What o.bind hands to hl.bind, one declaration at a time. The dispatcher is +# printed as the command string Hyprland would run, since that is where a web +# app bind carries the name it matches a window by. +bound() { + cat >"$tmpdir/bind.lua" <"$tmpdir/scan.lua" +[[ -s $tmpdir/scan.lua ]] || + fail "the menu's Lua bind scan can be read out of the script" + +home="$tmpdir/home" +mkdir -p "$home/.config/hypr" + +# The scan reads ~/.config/hypr/hyprland.lua. Pull in the one binding file this +# is about rather than the whole config: the rest of it is not what is under +# test. The two binds below are what a personal config adds, an id that is +# nothing like its label and a bind written straight against hl.bind. +cat >"$home/.config/hypr/hyprland.lua" <"$tmpdir/bind.lua" <&1 >/dev/null +} + +# What it writes on stdout, which belongs to the keybindings menu. +printed() { + declaration "$1" + lua "$tmpdir/bind.lua" 2>/dev/null +} + +reported=$(warnings 'o.bind("SUPER + K", "Calendar", "true", { locked = true }, { id = "calendar" })') +grep -qF 'SUPER + K' <<<"$reported" || + fail "a bind passing a fifth argument names itself in the warning" "$reported" +grep -qF 'passes 5' <<<"$reported" || + fail "a bind passing a fifth argument is reported" "$reported" +pass "a bind passing a fifth argument is reported" + +reported=$(warnings 'o.bind("SUPER + K", "Calendar", "true", { id = "calendar" })') +[[ -z $reported ]] || + fail "a bind passing four arguments is left alone" "$reported" +pass "a bind passing four arguments is left alone" + +# The menu loads the config under a stub and reads records off stdout, so a +# warning written there would arrive as a row rather than as a warning. +rendered=$(printed 'o.bind("SUPER + K", "Calendar", "true", { locked = true }, { id = "calendar" })') +grep -qx 'dispatcher: true' <<<"$rendered" || + fail "a bind that warns still dispatches what it declared" "$rendered" +! grep -qF 'passes 5' <<<"$rendered" || + fail "a warning stays off the stream the keybindings menu parses" "$rendered" +pass "a warning stays off the stream the keybindings menu parses" + +reported=$(warnings 'o.rebind("SUPER + K", "Calendar", "true", { locked = true }, { id = "calendar" })') +grep -qF 'SUPER + K' <<<"$reported" || + fail "a rebind passing a fifth argument is reported against its own keys" "$reported" +pass "a rebind passing a fifth argument is reported against its own keys" + +# Every wrapper that forwards to o.bind has to forward the extras with it, or it +# becomes the one place a fifth argument still disappears without a word. +reported=$(warnings 'o.bind_toggle("SUPER + CTRL + N", "Toggle nightlight", "nightlight", { locked = true }, { id = "nightlight" })') +grep -qF 'SUPER + CTRL + N' <<<"$reported" || + fail "a toggle bind passing a fifth argument is reported against its own keys" "$reported" +pass "a toggle bind passing a fifth argument is reported against its own keys" + +reported=$(warnings 'o.bind("SUPER + RETURN", "Terminal", { omarchy = "terminal", id = "Terminal" })') +grep -qF 'SUPER + RETURN' <<<"$reported" || + fail "an id inside a dispatcher table is reported" "$reported" +grep -qF 'id = "Terminal"' <<<"$reported" || + fail "an id inside a dispatcher table is quoted back in the warning" "$reported" +pass "an id inside a dispatcher table is reported" + +reported=$(warnings 'o.bind("SUPER + RETURN", "Terminal", { omarchy = "terminal" }, { id = "Terminal" })') +[[ -z $reported ]] || + fail "an id in the options table is where it belongs" "$reported" +pass "an id in the options table is where it belongs" + +# The dispatcher reads its own keys one at a time, so a stray id costs the bind +# nothing at the keypress. Only the id is lost, which is why it needs saying. +rendered=$(printed 'o.bind("SUPER + RETURN", "Terminal", { omarchy = "terminal", id = "Terminal" })') +grep -qx 'dispatcher: omarchy-launch-terminal' <<<"$rendered" || + fail "a dispatcher table carrying a stray id still dispatches" "$rendered" +pass "a dispatcher table carrying a stray id still dispatches" + +# command_from hands back a table whose keys it does not know, and Hyprland's own +# API is free to put anything in one. Warning there would be a guess. +reported=$(warnings 'o.bind("SUPER + K", "Opaque", { unknown_to_omarchy = true, id = "Opaque" })') +[[ -z $reported ]] || + fail "a dispatcher table Omarchy does not recognise is left alone" "$reported" +pass "a dispatcher table Omarchy does not recognise is left alone" + +# A warning nobody ever sees is the only acceptable outcome on a stock machine, +# so load every binding file Omarchy ships. Read the stub out of the menu rather +# than keeping a second copy of it: it is the one place the whole config loads. +sed -n "/^ lua <<'LUA'\$/,/^LUA\$/p" "$ROOT/bin/omarchy-menu-keybindings" | + sed '1d;$d' >"$tmpdir/scan.lua" +[[ -s $tmpdir/scan.lua ]] || + fail "the menu's Lua bind scan can be read out of the script" + +home="$tmpdir/home" +mkdir -p "$home/.config/hypr" + +cat >"$home/.config/hypr/hyprland.lua" </dev/null) +rm -f "$tmpdir/loaded" +reported=$(scan 2>&1 >/dev/null) + +[[ -n $scanned ]] || + fail "the scan reports the bindings Omarchy ships" "$reported" +[[ -f $tmpdir/loaded ]] || + fail "every binding file Omarchy ships loads under the scan" "$reported" +[[ -z $reported ]] || + fail "the bindings Omarchy ships warn about nothing" "$reported" +pass "the bindings Omarchy ships warn about nothing"