Warn when a bind's id lands where nothing reads it

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

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

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)

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" <<LUA
+hl = {
+  dsp = { exec_cmd = function(command) return command end },
+  bind = function(keys, dispatcher, opts)
+    print("description: " .. tostring(opts.description))
+    print("id: " .. tostring(opts.id))
+    print("dispatcher: " .. tostring(dispatcher))
+  end,
+}
+
+dofile("$ROOT/default/hypr/helpers.lua")
+
+$1
+LUA
+
+  lua "$tmpdir/bind.lua"
+}
+
+declared=$(bound 'o.bind("SUPER + K", "Calendar", "true")')
+grep -qx 'id: Calendar' <<<"$declared" ||
+  fail "a bind that names no id of its own answers to its description" "$declared"
+pass "a bind that names no id of its own answers to its description"
+
+declared=$(bound 'o.bind("SUPER + K", "Calendar", "true", { id = "calendar" })')
+grep -qx 'id: calendar' <<<"$declared" ||
+  fail "a declared id is the one the bind keeps" "$declared"
+grep -qx 'description: Calendar' <<<"$declared" ||
+  fail "a declared id leaves the description alone" "$declared"
+pass "a declared id is the one the bind keeps"
+
+# Nothing stops a config from handing one options table to several binds, and the
+# id fills in only when none is set yet. Write that fallback into the table the
+# caller passed and the first description becomes the id of every bind after it,
+# which is the one way an id can be wrong without anyone declaring it.
+declared=$(bound '
+local shared = {}
+o.bind("SUPER + K", "Calendar", "true", shared)
+o.bind("SUPER + M", "Mail", "true", shared)
+')
+grep -qx 'id: Mail' <<<"$declared" ||
+  fail "a reused options table does not keep the first bind's id" "$declared"
+pass "a reused options table does not keep the first bind's id"
+
+# The whole point of the id: this command's first argument is matched against an
+# open window's class or title, so it has to hold still while the label moves.
+declared=$(bound 'o.bind("SUPER + K", "Photos", { webapp = "https://photos.test/", focus = true }, { id = "Google Photos" })')
+grep -qF "omarchy-launch-or-focus-webapp 'Google Photos' 'https://photos.test/'" <<<"$declared" ||
+  fail "a web app bind matches its window by id" "$declared"
+! grep -qF "'Photos'" <<<"$declared" ||
+  fail "a web app bind does not match its window by description" "$declared"
+pass "a web app bind matches its window by id"
+
+declared=$(bound 'o.bind_toggle("SUPER + K", "Nightlight", "nightlight", { id = "nightlight" })')
+grep -qx 'id: nightlight' <<<"$declared" ||
+  fail "a toggle bind carries a declared id too" "$declared"
+pass "a toggle bind carries a declared id too"
+
+# 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. Read that scan out of
+# the menu rather than keeping a second copy of it here.
+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"
+
+# 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" <<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
+
+scanned=$(env -i PATH="$PATH" HOME="$home" OMARCHY_PATH="$ROOT" lua "$tmpdir/scan.lua")
+[[ -n $scanned ]] || fail "the scan reports the binds in the Lua config"
+
+# modmask, description, id, key, dispatcher kind, dispatcher arg
+[[ -z $(awk -F '\t' '$3 == "" { print }' <<<"$scanned") ]] ||
+  fail "every bind the scan reports answers to an id" "$scanned"
+pass "every bind the scan reports answers to an id"
+
+awk -F '\t' '$2 == "Terminal" && $3 == "Terminal" { found = 1 } END { exit !found }' <<<"$scanned" ||
+  fail "the scan reports the description as the id when no other was declared" "$scanned"
+awk -F '\t' '$2 == "Written against hl" && $3 == $2 { found = 1 } END { exit !found }' <<<"$scanned" ||
+  fail "a bind written against hl.bind still reports an id" "$scanned"
+pass "the scan reports the description as the id when no other was declared"
+
+awk -F '\t' '$2 == "Renamed later" && $3 == "Stable Id" { found = 1 } END { exit !found }' <<<"$scanned" ||
+  fail "the scan reports the id a bind declared, not its label" "$scanned"
+awk -F '\t' '$3 == "Stable Id" && $6 ~ /or-focus-webapp .Stable Id./ { found = 1 } END { exit !found }' <<<"$scanned" ||
+  fail "the scan reports the command the id built" "$scanned"
+pass "the scan reports the id a bind declared, not its label"
+
+awk -F '\t' '$3 == "WhatsApp" && $6 ~ /or-focus-webapp .WhatsApp./ { found = 1 } END { exit !found }' <<<"$scanned" ||
+  fail "a web app Omarchy ships focuses the window its id names" "$scanned"
+pass "a web app Omarchy ships focuses the window its id names"
+
+# A web app bind that focuses an open window spends its id on a pattern the web
+# app decides, so it cannot inherit one from a description somebody may later
+# translate. Anything added alongside the four has to say so out loud.
+while IFS= read -r binding; do
+  [[ $binding == *"id = "* ]] ||
+    fail "every web app bind that focuses an open window names its own id" "$binding"
+done < <(grep -rh 'webapp = ' "$ROOT/default/hypr/bindings" | grep -F 'focus = true')
+pass "every web app bind that focuses an open window names its own id"