Repository navigation
Conversation
f47f33f to
1137aba
Compare
Automated AI review
Outcome: When the double action is a shell helper that dispatches a registered shell shortcut ( Double action re-arms the single press on key release
Suggested change: run the double action from a 1 ms one-shot Reproducer (fails at 1137aba, model of Hyprland's release rule)Save as -- Run from the repository root: OMARCHY_PATH="$PWD" lua double-press-release-repro.lua
--
-- Same fake-Hyprland style as test/shell.d/hyprland-double-press-test.sh, plus
-- the one piece of Hyprland 0.56.2 key handling that test does not model:
--
-- * hl.dsp.global (also pass, send_shortcut, and the mouse forms of window
-- drag/resize) marks the bind being handled as releasePending when it is
-- dispatched during a key press (src/config/lua/bindings/
-- LuaBindingsDispatchers.cpp, dsp_global).
-- * On key release, a bind with releasePending counts as a special
-- dispatcher, so its Lua callback is called again instead of the release
-- being swallowed (src/managers/KeybindManager.cpp, handleKeybinds:
-- SPECIALDISPATCHER includes k->releasePending; the !pressed branch only
-- swallows "!k->release && !SPECIALDISPATCHER").
-- * Outside a key event (for example in an hl.timer callback) there is no
-- current keybind, and global sends a "pressed" event.
package.path = os.getenv("OMARCHY_PATH") .. "/?.lua;" .. package.path
local events, timers, clock = {}, {}, 0
local current_keybind = nil
local function expect_events(wanted, message)
local got, expected = table.concat(events, "|"), table.concat(wanted, "|")
if got ~= expected then
print("not ok - " .. message)
print(" expected: " .. expected)
print(" got: " .. got)
return false
end
print("ok - " .. message)
return true
end
local function advance(ms)
local target = clock + ms
while true do
local index, due
for i, timer in ipairs(timers) do
if timer.due <= target and (not due or timer.due < due.due) then
index, due = i, timer
end
end
if not due then break end
table.remove(timers, index)
clock = due.due
due.callback()
end
clock = target
end
hl = {
dsp = {
exec_cmd = function(command)
return { run = function() table.insert(events, "exec:" .. command) end }
end,
global = function(name)
return {
run = function()
if current_keybind then
current_keybind.release_pending = true
end
table.insert(events, "global:" .. name)
end,
}
end,
},
bind = function(keys, callback, options)
local handle = { keys = keys, callback = callback, options = options or {}, release_pending = false }
return setmetatable(handle, {
__index = { is_enabled = function() return true end },
__tostring = function() return "HL.Keybind(" .. keys .. ")" end,
})
end,
unbind = function() end,
timer = function(callback, options)
table.insert(timers, { callback = callback, due = clock + options.timeout })
end,
dispatch = function(action)
action.run()
end,
}
local function invoke(handle)
current_keybind = handle
handle.release_pending = false
if type(handle.callback) == "function" then
handle.callback()
else
hl.dispatch(handle.callback)
end
current_keybind = nil
end
-- Key press, then key release with the modifier still held.
local function tap(handle)
invoke(handle)
if handle.release_pending then
invoke(handle)
end
end
dofile(os.getenv("OMARCHY_PATH") .. "/default/hypr/helpers.lua")
local ok = true
-- Control: plain commands.
local commands = o.bind("SUPER + grave", "History", "omarchy-shell notifications showHistory", {
double_press = "omarchy-shell notifications dismissAll",
})
events, clock = {}, 0
tap(commands)
tap(commands)
advance(250)
ok = expect_events({ "exec:omarchy-shell notifications dismissAll" },
"a double press with command actions runs only the double action") and ok
-- The same shortcut written with Omarchy's shell helpers, which dispatch the
-- shell's registered global shortcuts (default/omarchy/shortcuts).
local helpers = o.bind("SUPER + grave", "History", { ipc = "notifications.showHistory" }, {
double_press = { ipc = "notifications.dismissAll" },
})
events, clock = {}, 0
tap(helpers)
tap(helpers)
advance(250)
ok = expect_events({ "global:omarchy:ipc.notifications.dismissAll" },
"a double press with shell-shortcut actions runs only the double action") and ok
os.exit(ok and 0 or 1)Output at 1137aba: With the double action moved into a 1 ms one-shot Details
Verified:
Optional notes:
More findings: new test depends on the working directory
Suggested change: add Run the new test from any directory other than the repository root, for example
Review informationTest scope: Source reading against Hyprland 0.56.2 and sandboxed Lua tests at 1137aba; not run in a live compositor. AI process: Opus 5.5 Medium coordination and synthesis, Opus 5.5 Xhigh technical review and final fact check, GPT 6 Sol Xhigh search for related issues, Opus 5.5 Medium editorial check. Opt out: To stop receiving these reviews, reply to this comment saying so. |
1137aba to
32f436d
Compare
|
@greptileai Please review the latest revision of this PR. |
|
Adds optional double-press actions to
o.bindando.rebind, allowing two actions on one shortcut without custom timer logic.The first press waits 250 ms by default. A second press within that window cancels the pending action and schedules
double_pressthrough a 1 ms one-shot timer. Dispatching outside the key event prevents global shortcuts from re-entering the binding on key release. Both actions accept the same commands, helpers, Lua functions, and dispatchers as ordinary bindings.Pending actions check that their binding is still enabled before running. The keybindings menu resolves timed shortcuts to their single action; arbitrary Lua callbacks require command metadata, as with ordinary bindings. Existing default shortcuts are unchanged.
Menu metadata uses weak callback keys so it does not keep removed callbacks alive.
Testing
Added coverage for timing boundaries, cancellation, triple presses, binding lifecycle, supported action types, callback collection, release-event re-entry, cancellation of deferred double actions, and resolving timed shortcuts in the keybindings menu. The focused double-press, keybindings-menu, default-config, and binding-conflict suites pass, along with Lua/Bash syntax and whitespace checks. Verified that an intentional Lua assertion failure causes the new test script to fail, and that the script passes when invoked from outside the repository root.
The revised helper passed ten automated cases in a running Hyprland 0.56.2 session using real timers, native binding handles, and command execution. A 1,000-cycle handle removal/replacement test retained only the live metadata entry and released it after removal. Physical keyboard testing confirmed single-press toggle and double-press clear before the metadata cleanup; the revised helper was retested through registered callbacks in the live compositor.
A physical keyboard comparison reproduced the release-event bug with the previous revision: a double press dispatched the double action and then the single action. With the revised helper, the same press/release sequence dispatched only the double action. The automated review's supplied reproducer also fails on the previous revision and passes on this one.
Known limitations
Hyprland 0.56.2 retains Lua callback registry references when using
hl.unbind; explicithandle:remove()releases them. This was reproduced using the native API without the Omarchy helper. Weak metadata keys avoid additional retention but do not fix the native behavior, including during repeatedo.rebindcalls.The keybindings-menu test uses a minimal configuration because the existing scanner failure tracked by #13276 prevents the full default configuration from reaching personal bindings. That fix is independent of the double-press implementation.