extra: pipe autoshift layer with HRM fold

[rgoulter]

Summary

Replaces per-key K.autoshift with an extra pipe that autoshift-izes a whole layer and folds existing HRM holds into a nested tap-hold.

• Nested tap-hold proof: K.A & hold (K.A&LeftShift & hold K.Ctrl & inner) works via NewPressedKey backdating. Outer profile timeout=200 Ignore → a / A; inner profile inner_hold {timeout=null, interrupt_response="HoldOnKeyPress"} → interrupt gives Ctrl.
• Pipe API: let AL = import "extra/autoshift_layer.ncl" in layers = [ base |> AL.autoshift, extra |> AL.autoshift ]
transform_key uses match {null, {hold,..tap_rest}, _} to fold HRM where present, otherwise k & hold (shifted k). Plain K.A → K.A & hold (K.A&LeftShift) (no HRM creation on plain keys). Transparent null, tap_dance etc. left as-is.

Commits

1. test: demonstrate nested tap-hold for autoshift HRM – ObservedKeymap with manually constructed nested Key via Key::tap_hold chain + backdate, covering outer tap, timeout-shifted, interrupt-Alt.
2. ncl: add extra pipe autoshift layer with HRM fold – ncl/extra/autoshift_layer.ncl (transform_key, autoshift, autoshift_with, autoshift_layer_keys) + Rust integration (autoshift_layer.rs: plain, timeout-shifted, interrupt HRM, reuses existing hold, layers pipe form).

Dropped prior per-key K.autoshift/K.autoshift_as + feature docs + tests (8459fd7, ef98c5d, bcf4a21) – not on the layer |> autoshift path.

Test plan

• just fmt + just check-quick (cargo fmt --check, cargo clippy -D warnings, cargo doc --deny warnings, ncl-format)
• nickel eval --import-path=ncl --field="evaluated_checks" ncl/checks.ncl → {}
• cargo test --test rust-integration -- nested_hold autoshift_layer (f2f0a42 + 47c7d8d)
• CI (GH Actions) – feat/autoshift → master (extra: pipe autoshift layer with HRM fold #671)

Notes

config.tap_hold.profiles.inner_hold must exist for the fold: {timeout=null, interrupt_response="HoldOnKeyPress"}.

Co-Authored-By: Muse Code powered by Meta Muse Spark muse-code@meta.com
Co-Authored-By: muse-spark-1.2 muse-spark@meta.com

[rgoulter]

Oh. I think tap-hold's hold can nest tap-hold; so it should be doable to express "uninterrupted longer press is shifted(of tap); interrupted longer press is modifier, and tap is unshifted tap".

[rgoulter]

Actual autoshift. Oh, neat.

[Copilot]

Pull request overview

Adds a new “extra pipe” Nickel helper to autoshift an entire layer while folding existing tap-hold/HRM holds into a nested tap-hold structure, and introduces Rust integration tests demonstrating nested tap-hold behavior plus the new layer |> AL.autoshift usage.

Changes:

• Added ncl/extra/autoshift_layer.ncl implementing transform_key, autoshift, autoshift_with, and keymap-layer rewriting utilities.
• Added Rust integration tests for nested tap-hold behavior and for the new autoshift-layer pipe API.
• Wired the new test modules into the existing Rust test suite module structure.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/rust/tap_hold/nested_hold.rs	New tests covering nested tap-hold behavior (outer timeout + inner HoldOnKeyPress profile).
tests/rust/tap_hold.rs	Registers the new nested_hold tap-hold test module.
tests/rust/keymap.rs	Registers the new autoshift_layer test module.
tests/rust/autoshift_layer.rs	New tests for extra/autoshift_layer.ncl behavior (plain, timeout-shifted, interrupt/HRM folding, and layers pipe form).
ncl/extra/autoshift_layer.ncl	New Nickel “extra” module implementing layer-wide autoshift with HRM fold and checks.

Suppressed comments (3)

ncl/extra/autoshift_layer.ncl:152

• K.autoshift is referenced in this check expectation, but keys.ncl doesn't provide that function anymore. Use the explicit autoshift expansion so the check can run.

      check_layer_pipe_is_autoshift = {
        actual = [K.A, K.B] |> autoshift,
        expected = [K.autoshift K.A, K.autoshift K.B],
      },

ncl/extra/autoshift_layer.ncl:158

• This check still expects K.autoshift K.B, but keys.ncl doesn't define autoshift anymore. Use the explicit autoshift expansion to keep the checks runnable.

        actual = [K.A & K.hold K.LeftCtrl, K.B] |> autoshift,
        expected = [
          K.A & K.hold (K.A & K.LeftShift & K.hold K.LeftCtrl & inner),
          K.autoshift K.B,
        ],

ncl/extra/autoshift_layer.ncl:177

• This expected value still uses K.autoshift, which is not provided by keys.ncl anymore. Expand autoshift explicitly so the Nickel checks compile/evaluate.

        expected = {
          keys = [K.A, K.B],
          layers = [[K.autoshift K.C, K.autoshift K.D]],
        },

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (4)

ncl/extra/autoshift_layer.ncl:30

• transform_key currently wraps every non-null, non-tap-hold key in a new tap-hold (via the default _ => k & K.hold ... arm). This contradicts the comment about leaving composites (tap_dance/sticky/…) alone and also changes behavior for keys like K.NO (key_code=0) and pure modifier keys, which should not become autoshifted tap-holds. Consider only autoshifting keyboard keys that actually have a non-zero key_code, and otherwise returning k unchanged.

        _ => k & K.hold (shifted_for k)

ncl/extra/autoshift_layer.ncl:20

• The transform_key docstring says the inner_hold profile must exist unconditionally, but inner_hold is only referenced when folding an existing {hold=..., ..} tap-hold. For plain-key autoshift (K.A -> K.A & hold (K.A & LeftShift)), this profile is not required, so the docs are currently stricter than the code.

        Inner profile must exist: `config.tap_hold.profiles.inner_hold = {timeout=null, interrupt_response="HoldOnKeyPress"}`.
        Outer uses the default profile (`timeout=200 Ignore` → `a`/`A`).

ncl/extra/autoshift_layer.ncl:72

• autoshift_layer builds all_layers = [base_keys] @ extra_layers, treating keymap.keys as layer 0 and keymap.layers as “extra layers”. But in ncl/keymap-ncl-to-json.ncl, keys is optional and layers is the canonical full layer list (defaulting to [keys]), so keymaps that set layers directly (and omit keys) will have their layer indices shifted and may also fail later on std.record.update "keys" because the field is absent. This helper should operate against the authored layers semantics (and only touch keys when it was present / being used as the base).

      let all_layers = [base_keys] @ extra_layers in

tests/rust/tap_hold/nested_hold.rs:114

• The Alt+B assertion assumes KC_B is always in HID slot r[2] and that the modifier byte is exactly MOD_LALT. This is more brittle than needed (and inconsistent with later checks using contains), and could fail if report ordering changes or additional modifiers are present.

    let has_alt = reports.iter().any(|r| r[0] == MOD_LALT);
    let has_b_with_alt = reports.iter().any(|r| r[0] == MOD_LALT && r[2] == KC_B);

[rgoulter]

Oh. ...
Oops.

This is one awkward part of the design.
It assumes inner-hold has "HoldOnKeyPress" interrupt response and no key press.

And this implementation implicitly assumes the name inner_hold huh?

That's enough for me to think this PR is still 'draft', or may be 'extra' rather than 'merge now'.

[rgoulter]

I think this is mergeable as-is.

Anonymous tap-hold behaviour profiles would make this smoother (since it wouldn't rely on inner_hold behaviour being defined). -- Can defer adding a Cucumber spec / .feature until then, maybe.

It's also natural to wonder about multi-case tap-hold definition rather than nesting a tap-hold within a tap-hold. I don't hate nesting tap-hold in tap-hold. But, a more natural way to express the behaviour is like tap & { uninterrupted_timeout = shifted_tap, interrupted = modifier }.

[rgoulter]

Ugh. muse didn't follow the agents.md instructions of "prefer squash/absorb rather than adding disorganised commits to address PR comments". (And the chat session was relatively fresh, so context bloat would be concerning).