[Bug] LT and macros not working together with Vial

pschonev commented 1 month ago

Hey there and first of all thank you for all the great work you are sharing! I am new to QMK (and Vial), so probably I am not understanding something obvious, but here is what I am experiencing:

I am using Achordion and it works great for the most part. Same-handed modifiers are not being activated and e.g. LSFT_T() is working as expected. Additionally, when I use MO(1) it will use my keys on layer 1 just fine, same for LT() for the most part.

However when I try to use LT(1, ...) on a key that has a macro "above it" on layer 0 it will never work, unless I am holding it down past the timeout. Instead it will just produce the macro from layer 0.

To give an example, I have LT(1, KC_SPACE) and a key that is this macro [["tap", "KC_QUOTE", "KC_SPACE"]] on layer 0 (useful for US International because KC_QUOTE is a dead key) and KC_GRAVE on layer 1. When I hold down LT(1, KC_SPACE) this will only ever produce KC_QUOTE until I reach the timeout.

It's the same for every macro I have on layer 0 and removing Achordion was the only thing that fixed it.

Is there any way I can change my config to deal with this?

Information

My config: https://github.com/pschonev/vial-qmk/tree/pschonev/sofle_choc/keyboards/sofle_choc/keymaps/pschonev

Do the keys involved use any of the following features?

[x] Achordion (from this repo)
[ ] Auto Shift
[ ] Combos
[ ] Key Overrides
[x] Mod-tap or Layer-tap keys
[ ] Other custom userspace code:

getreuer commented 1 month ago

Thanks for the report and clear description!

Bug outline

The way Achordion works is by intercepting hold events from QMK core on LT and MT keys, catching them as they pass through the process_record_user().

However, some events get handled before process_record_user(). For these events, Achordion is unable to process them as intended. Unfortunately, it does appear that is the case for Vial macros. In the Vial version of quantum.c, process_record_vial() is called shortly before process_record_kb() (which calls process_record_user()).

What we intend to happen is:

Key LT(1, KC_SPACE) is pressed.
QMK core sends a hold press event for LT(1, KC_SPACE). Achordion catches this event and blocks it from further processing. Since it was blocked, the switch to layer 1 hasn't happened (yet).
The key for the macro, but conceptually corresponding to KC_GRAVE, is pressed.
On seeing this event, Achordion settles the LT as held. It plumbs the hold press event, switching on layer 1. It then plumbs the latter key press, which now after the layer switch produces KC_GRAVE.

But what's happening is something like:

Key LT(1, KC_SPACE) is pressed.
QMK core sends a hold press event for LT(1, KC_SPACE). Achordion catches this event and blocks it from further processing. Since it was blocked, the switch to layer 1 hasn't happened (yet).*
The key for the macro, but conceptually corresponding to KC_GRAVE, is pressed.
THIS DIFFERS: process_record_vial() sees and processes the macro key event before Achordion can intercept it.

Possible workaround

It looks like a workaround to this issue is to implement the macro as a Vial custom keycode. Like this:

// Copyright 2024 Google LLC.
// SPDX-License-Identifier: Apache-2.0

#include "sendstring_us_international.h"

enum custom_keycode {
  M_QUOT = SAFE_RANGE,
  // More custom keys...
}

bool process_record_user(uint16_t keycode, keyrecord_t *record) {
  if (!process_achordion(keycode, record)) { return false; }

  switch (keycode) {
    case M_QUOT:
      if (record->event.pressed) { 
        SEND_STRING("'");
        // Or if SEND_STRING doesn't work, do
        // tap_code(KC_QUOT);
        // tap_code(KC_SPC);
      }
      return false;
    // More macros...
  }
  return true;
}

Then define and use M_QUOT in vial.json as described here.

Explanation: "M_QUOT" defines the name of the keycode (or name it whatever you like). In process_record_user(), we then handle when this custom keycode is pressed. IIUC, the SEND_STRING library should work with US International such that SEND_STRING("'") will correctly take into account that ' is a dead key provided the #include "sendstring_us_international.h" header. But if that doesn't work out, change the SEND_STRING line to tap_code(KC_QUOT); tap_code(KC_SPC); to implement it manually.

And critically, since we're now handling the macro, we can call process_achordion() just before the macro handler so that Achordion can intercept events when it should.

pschonev commented 1 month ago

Thanks a lot for the quick and comprehensive answer! This fixed the problem :)

I think there's a missing semicolon in their documentation at the end of the enum and you actually have to change M_QUOT = SAFE_RANGE to M_QUOT = SAFE_RANGE QK_KB_0 for this to work. SEND_STRING("'"); worked fine.

This also has the advantage that my macros now have proper names in the GUI instead of just M5.

getreuer / qmk-keymap