Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for the panic sequence to only consider physical key layout #929

Closed
ficcdaf opened this issue Feb 5, 2025 · 2 comments
Closed

Comments

@ficcdaf
Copy link

ficcdaf commented Feb 5, 2025

Note: when I refer to physical keys, I'm talking about the LHS of a mapping; the key that keyd detects before remapping it.

Motivation

It's mentioned in #315 that the panic sequence can be disabled by modifying the source code, but being able to disable the feature completely defeats the purpose of it. While I agree, I've found the behavior of triggering the panic sequence a bit inconsistent.

For example, the BS + ESC + ENTER sequence terminates keyd both when those physical keys (regardless of their remappings) are pressed -- this is the behaviour I'd expect. However, it also triggers when the "mapped output" resolves to that sequence as well. So, depending on the layout, there are a lot of different physical key sequences that can possibly resolve to the same "virtual sequence" and trigger the kill switch.

I argue that this behavior is inconsistent, and personally I've managed to trigger it accidentally on a few occasions.

Proposed Solution

I propose introducing a config option with a (tentative) name like kill_switch_physical_only=y/n. When the option is enabled, then keyd should only be terminated if the physical keys corresponding to the escape sequence are typed; if different keys that are mapped to keys corresponding to the escape sequence are pressed, it should be ignored.

I believe this feature would improve the usability and consistency of keyd without defeating the purpose of the panic sequence.

I propose the option should be disabled by default; s.t. keyd will behave exactly as it did before unless the user optionally enables this flag in the config.

Implementation

To the best of my knowledge, only the panic_check function in src/eventloop.c, as well as the functions that parse and process the config file would need to be changed -- so the implementation should not be very difficult.

If the maintainers think this would be a good feature to add but don't think it's high on the priority list, I can attempt to contribute the feature myself.

@rvaiya
Copy link
Owner

rvaiya commented Feb 20, 2025

Thanks for alerting me of this.

However, it also triggers when the "mapped output" resolves to that sequence as well. So, depending on the layout, there are a lot of different physical key sequences that can possibly resolve to the same "virtual sequence" and trigger the kill switch.

This should be considered a bug. It has been fixed on master.

I propose introducing a config option with a (tentative) name like kill_switch_physical_only=y/n. When the option is enabled, then keyd should only be terminated if the physical keys corresponding to the escape sequence are typed;

It has always been the intention for the physical sequence to exclusively terminate keyd. Remapped sequences should not count towards the panic sequence.

@ficcdaf
Copy link
Author

ficcdaf commented Feb 20, 2025

Thank you very much! I didn't realize it was a bug; I'm glad my report was helpful. Thanks for all your work on keyd!

meslubi2021 added a commit to Unity-for-manufacturing-assets-of-Unity/keyd that referenced this issue Mar 15, 2025
* man: Add note about config file exclusivity

* readme: Add explicit mention of wayland (rvaiya#559)

* man: Add note about maximum layer name length

* config: Allow 64 character layer names (rvaiya#558)

* Long overdue release

* docs: Fix changelog typos

* readme: Update arch package location

This has been imported into Arch's [extra] repository.

* make: Move service generation into install target (rvaiya#801)

* config: Increase the nested descriptor limit

* Enable GNOME extension for v47

* overloadi: Account for shifted symbols (rvaiya#875)

* Update README.md

since it says "Install AND start" adding `--now` might make more sense!

* keyd-application-mapper: Add support for the pop-os cosmic desktop

* keyd-application-mapper: Fix wlroots support for new windows

* readme: Combine systemctl `enable` and `start` commands by using `--now`

* readme: Fix broken QMK link

* ipc: Explicitly account for failure in the early stages of the connection (eliminates annoying compiler warning)

* evdev: Add support for AL_* family of keys found on some laptops

* listen: Eagerly terminate on pipe closure to accommodate more scripting use cases

* config: Make main a proper layout

* doc: Add Half-QWERTY layout to examples

This layout has been very useful to me for typing only with the
right hand. I did not test it for left-handed typing.

* doc: Add chromebook-linux.conf to examples

* keyd-application-mapper: Fix string escape bug causing issues on some platforms (rvaiya#649)

* layouts: Fix 0 in the FR layout

* core: Add special case for volume key devices in capabilities check

* doc: Add examples for common patterns.

* doc: Add additional key mappings to the chromeos example

* config: Increase macro size

* core: Improve support for thinkpad keyboards (rvaiya#905)

* layouts: Add graphite-angle-kp keyboard layout

* layouts: Add graphite keyboard layout

* core: Improve support for exotic mice

* core: Fix compilation on older systems

* Add support for scroll remapping

* doc: Add macos example to the readme

* Tweak readme example

* Improve mouse button support

* Add missing entries

` for the letter ḏal
Shift + ` for the shaddah
Shift + 0 for left parenthesis

* Add eastern Arabic zero

* core: Avoid interpreting remapped keys as part of a panic sequence (rvaiya#929)

* layout: Update graphite-angle-kp

Corrected a key, it was just a typo I guess.

ref : https://github.com/rdavison/graphite-layout

* doc: Update the application mapper man page

Clarifies that the verbose flag is needed to see window events in the output.

* keyd-application-mapper: Process SIGUSR1 on wayland (rvaiya#935)

* macro2: Fix timeout bug for nested macros

* doc: Update README

* keyd-application-mapper: Refactor wayland code

Clean up Wayland() and allow binding to multiple protocol objects simultaneously

* timeout: Add new behaviour when used as a tap target (rvaiya#879, rvaiya#738, rvaiya#944)

This patch expands the semantics of timeout() to cover the case in which it is
used as a tap target. This facilitates a number of novel use cases, like
discriminating between single/double tap or implementing per-key oneshot
timeouts.

Specifically timeout() is now defined in terms of arbitrary key events rather
than the behaviour of the key to which it is bound (if any). The new definition
executes the second action if no key events occur before the timeout expires.

The implication of this is that when timeout() is executed as the result of a
tap action, action2 will be executed after the timeout expires unless another
key is struck in the interval. Note that this is backward compatible with the
old definition, since a key up event (i.e a tap) will result in a resolution to
the first action if timeout() is directly bound to a key.

For Example:

	tab = overload(control, timeout(a, 100, b))

will presently produce no effect when 'tab' is tapped. Under the expanded
definition, tapping tab will produce 'b' if 100 milliseconds elapse without an
interceding key event.

* macro: Eagerly restore modifiers post execution (fixes rvaiya#947)

* macro: Ensure modifier state is properly tracked during macro execution

---------

Co-authored-by: Raheman Vaiya <[email protected]>
Co-authored-by: ainola <[email protected]>
Co-authored-by: birdbird <[email protected]>
Co-authored-by: Chris Schepman <[email protected]>
Co-authored-by: Pavel Slepushkin <[email protected]>
Co-authored-by: blankie <[email protected]>
Co-authored-by: Rajas Paranjpe <[email protected]>
Co-authored-by: diegorodriguezv <[email protected]>
Co-authored-by: Egor Pasko <[email protected]>
Co-authored-by: Merith <[email protected]>
Co-authored-by: Ridwan Mulyadi <[email protected]>
Co-authored-by: Fleefie <>
Co-authored-by: Garbaz <[email protected]>
Co-authored-by: Leandro M. Peralta <[email protected]>
Co-authored-by: Dick Marinus <[email protected]>
Co-authored-by: aljustiet <[email protected]>
Co-authored-by: Justin <[email protected]>
Co-authored-by: Awelaa <[email protected]>
Co-authored-by: Cédric <[email protected]>
Co-authored-by: Robert Benson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants