fix(gpu): F-004 AltGr strip → Ctrl+Alt only; run pmacs-protocol tests in CI
Two follow-ups on this session's audit work, both on freshly-merged code. F-004 hardening (regression fix). The AltGr text-input strip (`is_layout_text`) gated on *any* command modifier, so it fired on Alt-alone. On macOS the Option key is reported as Alt and emits printable text for most letters (Option+x → "≈"), so every GUI Meta binding (M-x, M-f, M-b, …) was stripped to a self-insert. Tighten the gate to the true AltGr signature — both Ctrl and Alt (the LCtrl+RAlt the OS synthesizes on Windows) — so Alt-alone forwards as a Meta chord again while Windows AltGr still inserts. Strict narrowing of when we strip: no-op on Linux/Windows, unblocks macOS Option-as-Meta. Test flips the Alt-alone € assertion and adds the macOS Option+x case. CI coverage (F-001 residue). `workspace_default_members` is only the root `pmacs` package, so `cargo test` skipped pmacs-protocol — the shared wire format the daemon, TUI, and GPU all depend on. Add `cargo test -p pmacs-protocol --all-targets` to the test job (its ~12 encode/decode + transport-framing tests). All three first-party crates now run in CI (root pmacs, pmacs-gpu via the render job, now pmacs-protocol). Validated: fmt clean; clippy -p pmacs-gpu --all-targets clean; pmacs-gpu tests 42 pass (render tests on the local adapter, PMACS_REQUIRE_GPU=1); pmacs-protocol 12 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014TXbAwk27agwhrNNrhLi2U
This commit is contained in:
parent
6acbb98e38
commit
623bd884af
|
|
@ -92,6 +92,13 @@ jobs:
|
||||||
# to the behavior under test.
|
# to the behavior under test.
|
||||||
- run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
|
- run: cargo test --all-targets --no-default-features --features ${{ matrix.lua }} -- --test-threads=1
|
||||||
- run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
|
- run: cargo test --doc --no-default-features --features ${{ matrix.lua }}
|
||||||
|
# The workspace default member is only the root `pmacs` package, so
|
||||||
|
# the runs above never execute pmacs-protocol's own tests — the
|
||||||
|
# shared wire format (encode/decode, transport framing) the daemon,
|
||||||
|
# TUI, and GPU all depend on. Run them explicitly (Lua-flavor
|
||||||
|
# independent, so once is enough; runs on every matrix leg cheaply
|
||||||
|
# off the shared cache).
|
||||||
|
- run: cargo test -p pmacs-protocol --all-targets
|
||||||
|
|
||||||
acceptance:
|
acceptance:
|
||||||
name: M1 Acceptance Gates
|
name: M1 Acceptance Gates
|
||||||
|
|
|
||||||
|
|
@ -78,6 +78,15 @@ commands, e.g. `cargo clippy --workspace --all-targets -- -D warnings` and
|
||||||
`default-members` if the intent is for plain `cargo test` and `cargo clippy` to
|
`default-members` if the intent is for plain `cargo test` and `cargo clippy` to
|
||||||
cover all first-party crates.
|
cover all first-party crates.
|
||||||
|
|
||||||
|
Resolution (PR #75, then extended): the three `pmacs-gpu` lints were fixed and
|
||||||
|
CI now runs `cargo clippy -p pmacs-gpu --all-targets` (README uses `--workspace`).
|
||||||
|
That closed the *clippy* blind spot but not the symmetric *test* one — the
|
||||||
|
default-member scope also means `cargo test` skips `pmacs-protocol`. F-014's
|
||||||
|
GPU-render job later ran `pmacs-gpu`'s tests; this change adds
|
||||||
|
`cargo test -p pmacs-protocol --all-targets` to the test job so the shared wire
|
||||||
|
format (its ~12 encode/decode + transport-framing tests) is finally exercised in
|
||||||
|
CI. All three first-party crates now run in CI.
|
||||||
|
|
||||||
### F-002 - Medium - `--all-features` is not buildable because Lua features are mutually exclusive
|
### F-002 - Medium - `--all-features` is not buildable because Lua features are mutually exclusive
|
||||||
|
|
||||||
Evidence:
|
Evidence:
|
||||||
|
|
@ -153,6 +162,19 @@ text input from command chords. Add tests for AltGr-style printable input. If th
|
||||||
wire protocol needs it, add an AltGraph modifier bit rather than folding it into
|
wire protocol needs it, add an AltGraph modifier bit rather than folding it into
|
||||||
Ctrl/Alt.
|
Ctrl/Alt.
|
||||||
|
|
||||||
|
Resolution (PR #76, then hardened): a keypress that produced printable
|
||||||
|
`key.text` while a command modifier was held is reclassified as text input and
|
||||||
|
the modifiers are stripped (`is_layout_text`). The first cut gated on *any*
|
||||||
|
command modifier, which over-reached: on macOS the `Option` key is reported as
|
||||||
|
`Alt` and emits printable text for most letters, so `Option+x` was stripped to a
|
||||||
|
plain insert — swallowing every GUI Meta binding (`M-x`, `M-f`, …) on that
|
||||||
|
platform. The gate is now the true AltGr signature — **both `Ctrl` and `Alt`**
|
||||||
|
(the LCtrl+RAlt the OS synthesizes for AltGr on Windows) — so `Alt`-alone forwards
|
||||||
|
as a Meta chord again while Windows AltGr still inserts. A protocol AltGraph bit
|
||||||
|
was *not* added; the text-presence heuristic is sufficient and layout-agnostic.
|
||||||
|
Not locally validatable on macOS (no box) — a narrowing of when we strip, so
|
||||||
|
low-risk on Linux/Windows.
|
||||||
|
|
||||||
### F-005 - Medium - Package install paths collide for namespaced packages with the same basename
|
### F-005 - Medium - Package install paths collide for namespaced packages with the same basename
|
||||||
|
|
||||||
Evidence:
|
Evidence:
|
||||||
|
|
|
||||||
|
|
@ -800,13 +800,14 @@ impl ApplicationHandler<AppEvent> for App {
|
||||||
|
|
||||||
// AltGr / international text (audit F-004). winit reports
|
// AltGr / international text (audit F-004). winit reports
|
||||||
// the text a keypress produces; when a keypress yields
|
// the text a keypress produces; when a keypress yields
|
||||||
// printable text *while* Ctrl/Alt is held — AltGr is
|
// printable text *while both Ctrl and Alt* are held — the
|
||||||
// Ctrl+Alt on Windows and some layouts — it's text input,
|
// AltGr signature on Windows (LCtrl+RAlt) — it's text
|
||||||
// not a command chord. Strip the Ctrl/Alt (keep Shift) so
|
// input, not a command chord. Strip those modifiers (keep
|
||||||
// it inserts (through the plain-text path, or the daemon's
|
// Shift) so it inserts (through the plain-text path, or the
|
||||||
// SelfInsert while a prompt is open) instead of being
|
// daemon's SelfInsert while a prompt is open) instead of
|
||||||
// routed to the keymap. On platforms where AltGr isn't
|
// being routed to the keymap. Alt alone is left intact so
|
||||||
// Ctrl/Alt this is a no-op (the chord was already plain).
|
// macOS Option-as-Meta still reaches the keymap; on layouts
|
||||||
|
// where AltGr isn't Ctrl+Alt this is a no-op.
|
||||||
if matches!(pkey, ProtocolKey::Char(_))
|
if matches!(pkey, ProtocolKey::Char(_))
|
||||||
&& is_layout_text(key.text.as_deref(), pmods)
|
&& is_layout_text(key.text.as_deref(), pmods)
|
||||||
{
|
{
|
||||||
|
|
@ -4982,16 +4983,25 @@ fn is_command_chord(key: ProtocolKey, mods: Modifiers) -> bool {
|
||||||
) && (mods.contains(Modifiers::CTRL) || mods.contains(Modifiers::ALT))
|
) && (mods.contains(Modifiers::CTRL) || mods.contains(Modifiers::ALT))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether a keypress is **layout text input carried under a command
|
/// Whether a keypress is **`AltGr` layout text** (audit F-004): winit
|
||||||
/// modifier** — the `AltGr` case (audit F-004): winit produced printable
|
/// produced printable `text` while **both `Ctrl` and `Alt`** are held.
|
||||||
/// `text` while `Ctrl`/`Alt` is held. On layouts/platforms where `AltGr`
|
/// `AltGr` is `Ctrl+Alt` on Windows (the OS synthesizes LCtrl+RAlt), and
|
||||||
/// is `Ctrl+Alt` (Windows), the produced character would otherwise be
|
/// on such layouts the produced character (`@`, `€`, `{`, …) would
|
||||||
/// misclassified as a command chord; when this is true the caller strips
|
/// otherwise be misclassified as a command chord; when this is true the
|
||||||
/// the command modifiers so it inserts. Returns `false` for genuine
|
/// caller strips the command modifiers so it inserts.
|
||||||
/// command chords (which produce no text, or a control char) and for
|
///
|
||||||
/// plain text (no command modifier — already handled).
|
/// The gate is deliberately `Ctrl+Alt`, **not** "any command modifier":
|
||||||
|
/// `Alt` alone is *not* `AltGr`. On macOS the `Option` key is reported as
|
||||||
|
/// `Alt` and produces printable text for most letters (`Option+x` → "≈"),
|
||||||
|
/// but Option-as-Meta is exactly how the GUI reaches `M-x` / `M-f` / … —
|
||||||
|
/// stripping `Alt`-alone would swallow every macOS Meta chord. Genuine
|
||||||
|
/// `AltGr` needs both modifiers, so requiring both leaves `Alt`-alone
|
||||||
|
/// (macOS `Option`, plain `Meta`) to forward as command chords. Returns
|
||||||
|
/// `false` for genuine command chords (no text, or a control char) and
|
||||||
|
/// for plain text (no command modifier — already handled).
|
||||||
fn is_layout_text(text: Option<&str>, mods: Modifiers) -> bool {
|
fn is_layout_text(text: Option<&str>, mods: Modifiers) -> bool {
|
||||||
!is_plain_text_modifiers(mods)
|
mods.contains(Modifiers::CTRL)
|
||||||
|
&& mods.contains(Modifiers::ALT)
|
||||||
&& text.is_some_and(|t| !t.is_empty() && t.chars().all(|c| !c.is_control()))
|
&& text.is_some_and(|t| !t.is_empty() && t.chars().all(|c| !c.is_control()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -6074,8 +6084,14 @@ mod tests {
|
||||||
// text; that's text input, so the caller strips the modifiers.
|
// text; that's text input, so the caller strips the modifiers.
|
||||||
let ctrl_alt = Modifiers::CTRL | Modifiers::ALT;
|
let ctrl_alt = Modifiers::CTRL | Modifiers::ALT;
|
||||||
assert!(is_layout_text(Some("@"), ctrl_alt));
|
assert!(is_layout_text(Some("@"), ctrl_alt));
|
||||||
assert!(is_layout_text(Some("€"), Modifiers::ALT));
|
|
||||||
assert!(is_layout_text(Some("{"), ctrl_alt));
|
assert!(is_layout_text(Some("{"), ctrl_alt));
|
||||||
|
assert!(is_layout_text(Some("€"), ctrl_alt)); // AltGr+e on many layouts
|
||||||
|
// Alt ALONE is not AltGr. On macOS the Option key is Alt and emits
|
||||||
|
// printable text (Option+x → "≈"), but Option-as-Meta is how the
|
||||||
|
// GUI reaches M-x / M-f — leave it intact so it forwards as a
|
||||||
|
// command chord instead of self-inserting the symbol.
|
||||||
|
assert!(!is_layout_text(Some("≈"), Modifiers::ALT)); // macOS Option+x → M-x
|
||||||
|
assert!(!is_layout_text(Some("€"), Modifiers::ALT));
|
||||||
// Genuine command chords produce no text (or a control char) —
|
// Genuine command chords produce no text (or a control char) —
|
||||||
// not layout text, so they still route to the keymap.
|
// not layout text, so they still route to the keymap.
|
||||||
assert!(!is_layout_text(None, Modifiers::CTRL)); // C-a etc.
|
assert!(!is_layout_text(None, Modifiers::CTRL)); // C-a etc.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue