Bug: Transparent keys on base layer act as dead keys about kanata HOT 7 CLOSED

Thanks!

While I can think of some potential fixes for the behaviour, they don't seem particularly elegant to me.

With the way kanata is designed, the processing loop handles very little logic involved with what the active layer's mappings are. The keyberon library does most of the work for this. Having the defsrc declaration as a catch-all layer below all of the other layers also doesn't play well into the keyberon library's current feature set.

Is there a reason other than convenience while writing the configuration file to have transparency in the possible base layers that you would use? Given that a configuration file is used much more frequently than it is written/modified, my current preference is to document this as a known difference between how kmonad and kanata behave. I'm open to changing the behaviour if there's a compelling reason though.

from kanata.

This doesn't strike me as a processing loop change, but more of a parsing change. When an underscore is used on the base layer, it should be read as the literal key referenced in the defsrc. That said I'm not super familiar with the codebase yet. If this is something you're hesitant about from a technical standpoint, I'd be happy to dig in and send a PR with more detail.

From a design standpoint, however, I don't think you should discount convenience while writing the configuration file. For most users, this is literally the only way they will interact with Kanata. The "human readable configuration file" is the very first feature listed on the Readme, and is likely the most important.

It might be better to frame this as clarity rather than convenience. When scanning a config file, a base layer with only a few functions defined (like media controls, or home row mods) is immediately clear when those are the only nontransparent keys. Otherwise a reader would need to search through the entire keyboard looking for differences from the defsrc block. This clarity helps new users (and potential contributors) onboard faster, and helps avoid frustrating mistakes which might have seasoned users looking for other projects.

from kanata.

I hadn't considered doing it in the parsing stage. Parsing and setting up layer functionality is only done once at the beginning and otherwise only for live reload (which currently leaks memory) due to how the keyberon library works. With this in mind, consider the following sequence (which is somewhat nonsensical, but is possible):

1. layer-switch to A which has transparency (transparent keys delegate to defsrc)
2. layer-switch to B that has no transparency
3. layer-toggle to A (transparent keys now delegate to layer B)

Due to how kanata interacts with the keyberon library, switching behaviours between steps (1) and (3) is not feasible for a single layer. However, while writing this I thought of an idea. Every layer can have two versions; one where it is the default and delegates to defsrc (1) and one where it uses transparent keys as expected (3). The target layer for layer-switch would be (1) and the target layer for layer-toggle would be (3).

I think this works out to be a pretty decent solution for how kanata works.

If you have other ideas or want to try implementing this solution, PRs are welcome. Otherwise I'll get around to it probably within a week.

from kanata.

A fix has been merged into main. I believe it's fixed based on the added test, though I haven't done any manual tests due to lack of time.

I'll be doing manual tests within 24h.

from kanata.

I built this on Windows, and it seems now any layer-toggle is broken. Here are the logs I get for the config in the repro section above, just tapping the a key:

05:12:11 [INFO] Kanata: config parsed
05:12:11 [INFO] Kanata: entering the processing loop
05:12:11 [INFO] Init: catching only releases and sending immediately
05:12:12 [INFO] Starting kanata proper
05:12:15 [DEBUG] (2) kanata::kanata: event loop: KeyEvent { code: KEY_A, value: Press }
thread '<unnamed>' panicked at 'index out of bounds: the len is 2 but the index is 3', src\kanata.rs:235:42
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\std\src\panicking.rs:584
   1: core::panicking::panic_fmt
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:143
   2: core::panicking::panic_bounds_check
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:85
   3: kanata::kanata::Kanata::handle_time_ticks
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
05:12:15 [DEBUG] (2) kanata::kanata: event loop: KeyEvent { code: KEY_A, value: Release }
thread 'kanata' panicked at 'failed to send on channel: "Disconnected(..)"', src\kanata.rs:386:17
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\std\src\panicking.rs:584
   1: core::panicking::panic_fmt
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:143
   2: core::ops::function::FnOnce::call_once{{vtable.shim}}
   3: kanata::oskbd::windows::hook_proc
   4: GetDlgCtrlID
   5: CreateSystemThreads
   6: KiUserCallbackDispatcher
   7: NtUserGetMessage
   8: GetMessageW
   9: native_windows_gui::win32::dispatch_thread_events
  10: kanata::kanata::Kanata::event_loop
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

from kanata.

Ah right, the layer printing needed to be adjusted to match the new double layer approach. Fixed in the latest main.

from kanata.

As far as I can tell, this issue should be fixed in the latest main branch. Please feel free to re-open if you find issues.

from kanata.