Detect lua special dispatchers by reference#15215
Conversation
A pass/global/send_shortcut/mouse dispatcher declared from the Lua config is registered under handler == "__lua", so CKeybindManager cannot tell it is "special" up front. These dispatchers instead set k->releasePending from *inside* the dispatch call - after the press-time `else if (SPECIALDISPATCHER && pressed)` add that records native special dispatchers in m_pressedSpecialBinds. The Lua bind therefore never lands in that set, so on release SPECIALTRIGGERED (and thus IGNORECONDITIONS) is false, the modmask guard in the selection loop skips the bind for a modifier key, and the release is never forwarded - so a key bound to a modifier (e.g. push-to-talk on Right-Control) latches "on". Back-fill the tracking right after the dispatch call when a bind marked itself special via releasePending. The !SPECIALDISPATCHER guard keeps this mutually exclusive with the top-of-loop add (so a releasePending value that leaked across a held key does not double-add), and !SPECIALTRIGGERED avoids re-adding an already-tracked bind. Fixes all five releasePending-setting Lua dispatchers, not just pass. Tests (hyprtester, keyboard-modifiers recorder client): - luaPassForwardsModifierRelease: modifier pass forwards both edges (the regression; fails before the fix) - luaPassForwardsNonModifierRelease: non-modifier pass still works (over-correction guard) - luaSendShortcutForwardsModifierRelease: the fix generalizes to send_shortcut - luaPassModifierReleaseWithHeldKey: exactly one release forwarded per cycle while another key is held (the leaked-releasePending path)
The pass/global/send_shortcut/mouse dispatchers must be released the instant
their key goes up, ignoring modifier state. CKeybindManager recognizes the
native ones by their handler string and tracks them in m_pressedSpecialBinds on
press, so the release bypasses the modmask guard (IGNORECONDITIONS). A Lua
dispatcher is registered under handler == "__lua", so it can't be recognized
that way; instead it flagged itself by setting k->releasePending from inside the
dispatch call. That runs after the press-time tracking, so the bind was never
added to m_pressedSpecialBinds, and on release of a modifier key the guard
dropped the event - a key bound to a modifier (e.g. push-to-talk on
Right-Control) latched "on".
Recognize the wrapped dispatcher up front instead. At hl.bind, lua_tocfunction
sees through the hl.dsp.* closure to the underlying C dispatcher, so a bind
whose dispatcher is pass/global/send_shortcut/mouse-drag/resize is flagged
SKeybind::wrapsSpecialDispatcher and folded into SPECIALDISPATCHER - tracked on
press and released on key-up identically to a native bind. The
m_pressedSpecialBinds / IGNORECONDITIONS machinery is unchanged; only the source
of a Lua bind's special-ness moves from the mid-dispatch releasePending flag
(now removed) to a bind-time check.
Recognition is by dispatcher identity, so it covers hl.bind('x', hl.dsp.pass{...})
but not a plain Lua function that itself calls hl.dispatch(hl.dsp.pass{...}),
which is intentionally unsupported / not treated specially.
Tests (hyprtester, keyboard-modifiers recorder client):
- luaPassForwardsModifierRelease: modifier pass forwards both edges (the regression)
- luaPassForwardsNonModifierRelease: non-modifier pass forwards both edges
- luaSendShortcutForwardsModifierRelease: covers send_shortcut too
- luaPassModifierReleaseWithHeldKey: exactly one release per cycle with another key held
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Hmm, that's odd, I ran clang-format. 
|
|
Oh, duh, the example command lists files from src/ and I edited some things under hyprtester. |
|
Build failure seems unrelated. |
|
@codex review please |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a60aa02e0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // detect a wrapped native-special dispatcher now, while the dispatcher function is still on the stack | ||
| // (luaL_ref below pops it). Lets the bind be tracked/released like a native pass/global/send_shortcut/mouse. | ||
| kb.wrapsSpecialDispatcher = Internal::dispatcherFunctionIsSpecial(L, -1); |
There was a problem hiding this comment.
When the bind target is a normal Lua function, dispatcherFunctionIsSpecial returns false even if that function later calls hl.dispatch(hl.dsp.pass(...)) or send_shortcut/global/mouse. Since this commit also removes the old releasePending marking from those dispatcher implementations, a config such as hl.bind('X', function() hl.dispatch(hl.dsp.pass({ window = '...' })) end) will now fire the special dispatcher on press but be treated as an ordinary non-release bind on key-up, so the release edge is suppressed instead of forwarded; this regresses existing function-wrapped Lua binds that the previous m_currentKeybind->releasePending path covered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Even sounds like a common usecase to bind it like this and conditionally pass some ptt key only when gaming/fullscreen or something.
There was a problem hiding this comment.
I mentioned in bullet 2 on commit 2 that this will not work on non-direct (e.g., nested) dispatcher binds.
I'm not sure that this is a regression because releasePending doesn't seem to be working... the root cause is releasePending is set too late by the dispatcher. We'd either need to recompute everything releasePending might have affected (possible, but messy) or lift the value earlier in the lifecycle, of which this PR is one way.
If the direct Lua bind detection is too restrictive, we could add a Boolean somewhere on hl.bind(..., special dispatcher=true)?
There was a problem hiding this comment.
well it has to either work or get ignored, we can't let pass just fuck up the key starte
There was a problem hiding this comment.
To clarify, you're saying the nested dispatcher case is a requirement? In that case, do you approve of the argument to bind approach or not?
Is there an alternative you would prefer? Another option might be to look at what it would mean to consider all bindings special in this way... tbh I'm responding right now because I can't sleep but I'm also too tired to think it through so I'll consider the implications for that tomorrow.
There was a problem hiding this comment.
Yes, hl.dispatch with a pass inside a bind should work properly, or be ignored. It's fine to ignore it too because we have send_shortcut.
Argument is not acceptable. This should be automatic.
4 participants
Describe your PR, what does it fix/add?
This attempts to fix #14417, where the
hl.dsp.passdispatcher does not function the same as the previous nativepassdispatcher. e.g.,In practice, this has manifested for me personally as the Push-to-talk activating, but never deactivating. So, PRESSED, but not RELEASED.
There are actually two fixes implemented here that I want to discuss which is preferred (or neither):
Commit 1 (9cbf850)
k->releasePendingmachinery by backfillingm_pressedSpecialBindsCommit 2 (ce1d603)
hl.bind("x", function() hl.dispatch(hl.dsp.pass(...)) end). Feature?SDispatcherRef. This is easy to add, if desired.Oh, and I added some tests in Commit 1 - even if we go with the approach from Commit 2, they're useful.
Commit 1 is narrower but hackier. Following the logic is difficult.
Commit 2 is a broader change but more aligned with how the prior generation of special dispatchers worked. I much prefer this version, but maybe I'm missing something and there is a good reason it was first implemented with
k->releasePending.Root Cause
SPECIALDISPATCHER special cases several bind dispatchers. When the bind is in lua, the handler is
__lua, notpass(as an example). The current code attempts to fix this by settingreleasePendingin the special dispatcher lua binding (e.g., dsp_pass). But, if I'm reading this correctly, the DISPATCHER isn't called until deep inside the loop, well after the SPECIALDISPATCHER is computed.More succinctly, we check a condition at the top of the loop, but the condition variable is set INSIDE the dispatcher after that check is made.
Is it ready for merging, or does it need work?
Man, to quote last time (PR unrelated):
Alright, it's not about multiple monitors this time but it does involve running discord inside an XWAYLAND session inside a debug build and I don't have confidence that that layering will be indicative one way or another. If there was a convenient way to restart my login session with the debug build, I'd do that.
AI Disclosure
I used Claude, particularly for the root causing, but gave it instructions on solutions. I have taken the time to understand the root cause and both solutions. I am a human writing this PR and you will only engage with me, a real human bean.