From f300b77533bf1c5008a491d670546c99eb7b249d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 14 Jul 2026 15:02:59 +0100 Subject: [PATCH] =?UTF-8?q?fix(highlight):=20PR=20#116=20round=201=20?= =?UTF-8?q?=E2=80=94=20shebang=20precedence,=20pinned=20grammar,=20env=20o?= =?UTF-8?q?perands?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings, all in builtin/runtime/syntax.lua. 1. [P1] Syntax bypassed extension precedence and grammar availability. attach_for_active_buffer resolved `language_for_path or shebang`, but language_for_path knows only grammar-backed extensions — so a `.py` file opening with `#!/bin/sh` fell through to the shebang and got a bash parse tree, and an extensionless `#!/usr/bin/env python3` script dispatched "python" (no grammar) and raised "unknown language". A new resolve_active_language walks the full precedence chain — grammar extension -> LSP filetype map -> shebang — consulting the shebang only when the extension is unrecognized (a recognized non-grammar extension like .py is authoritative). Dispatch is then gated on pmacs.parse._has_language(lang), so grammarless languages are skipped silently. The extension parts stay keyed on buf:name() (unchanged from before), so path-less buffers that resolve a grammar by name — e.g. generated markdown buffers — are unaffected. 2. [P2] Editing an open script's shebang left parsing/highlighting stale. The after-edit path re-sniffed the mutable shebang: sh -> python raised "unknown language" while leaving the old bash tree, and sh -> lua swapped the parse tree under a highlight overlay still holding the original grammar's query. Reparse now uses the language pinned at first attach (parse_lang_by_buffer), never re-resolving — a language change needs a close/reopen, as it does for extensions. 3. [P2] `env` options with operands were mistaken for interpreters. `#!/usr/bin/env -u FOO python3` skipped `-u` but took `FOO`. The env walk now skips the operand of the operand-consuming GNU-env options (-u/--unset, -C/--chdir, -a/--argv0) before selecting the interpreter. -S/--split-string stays excluded (its string carries the interpreter). Tests (bite-verified against pre-fix syntax.lua — each fails without its fix; scripts/bite HEAD builtin/runtime/syntax.lua): - m4_shebang_does_not_override_extension now also asserts _has_view is false (no bash grammar tree for a `.py` + `#!/bin/sh`), not only the LSP language. - m4_shebang_extensionless_grammarless_language_is_silent — extensionless python resolves for LSP, gets no grammar view, and records no error. - m4_shebang_edit_keeps_pinned_grammar — rewriting a `#!/bin/sh` script's shebang to lua keeps the bash tree and reports no error. - m4_shebang_resolver_maps_interpreters — added the env-operand cases (`-u FOO`, `-C /tmp`, combined). Gates: fmt; clippy -D warnings; m4_acceptance --skip basedpyright; GPU; git diff --check all green. The only sweep failure is the pre-existing editor::composition_overhead_under_ten_percent render microbenchmark (ratio hovers at the 1.10 cutoff; flakes ~1/3 even isolated single- threaded, already asserted-off on macOS) — a pure-Rust render loop this Lua-only change cannot touch. Change is Lua-only plus the acceptance tests. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01YJ9FQ832QwftJXCD9LeFan --- builtin/runtime/syntax.lua | 71 ++++++++++++++++---- tests/m4_acceptance.rs | 128 +++++++++++++++++++++++++++++++++++-- 2 files changed, 182 insertions(+), 17 deletions(-) diff --git a/builtin/runtime/syntax.lua b/builtin/runtime/syntax.lua index ae73b76..cfe71ed 100644 --- a/builtin/runtime/syntax.lua +++ b/builtin/runtime/syntax.lua @@ -96,11 +96,29 @@ function pmacs.parse.language_from_shebang(buf) local base = first:match("([^/]+)$") or first if base == "env" then base = nil + local seen_env = false + local skip_next = false for tok in rest:gmatch("%S+") do - -- Skip the `env` path itself, its flags (`-S`, `--split-string`), - -- and inline `VAR=value` assignments; the first bare word left is - -- the real interpreter. - if tok ~= first and tok:sub(1, 1) ~= "-" and not tok:find("=", 1, true) then + if not seen_env then + seen_env = true -- the `env` path token itself + elseif skip_next then + skip_next = false -- the operand consumed by the previous option + elseif tok:find("=", 1, true) then + -- `VAR=value` env assignment, or a `--long=value` option: both + -- self-contained, skip. + elseif tok:sub(1, 1) == "-" then + -- An option. A few GNU-env short options and their long forms + -- consume the *next* token as an operand (`-u NAME`, `-C DIR`, + -- `-a NAME`); skip that operand too, or its value is mistaken for + -- the interpreter. `-S`/`--split-string` is deliberately absent: + -- the string it introduces contains the interpreter, which the + -- walk then picks up. An option with an attached operand + -- (`-uNAME`) is one self-contained token and needs no skip. + if tok == "-u" or tok == "-C" or tok == "-a" + or tok == "--unset" or tok == "--chdir" or tok == "--argv0" then + skip_next = true + end + else base = tok:match("([^/]+)$") or tok break end @@ -116,14 +134,37 @@ end -- attach; the kill path clears the entry below if/when it lands. local highlighted_buffers = {} +-- Filetype-aware language resolution for the active buffer, in +-- precedence order: grammar extension → LSP filetype map → shebang. The +-- shebang is consulted ONLY when the extension is unrecognized (a known +-- non-grammar extension like `.py` must not fall through to a stray +-- `#!/bin/sh` and be misparsed as bash). Keyed on `buf:name()` for the +-- extension parts (matching the historical behavior — path-less buffers +-- that resolve a grammar by name keep working); the shebang reads buffer +-- content directly. +local function resolve_active_language(buf) + local name = buf:name() + if name then + local grammar = pmacs.parse.language_for_path(name) + if grammar then return grammar end + local ext = name:match("%.([%w_]+)$") + local by_ext = ext and pmacs.lsp and pmacs.lsp.filetypes and pmacs.lsp.filetypes[ext] + -- A recognized (even non-grammar) extension is authoritative; do not + -- consult the shebang for it. + if by_ext then return by_ext end + end + return pmacs.parse.language_from_shebang(buf) +end + local function attach_for_active_buffer() local buf = pmacs.window.buffer() if not buf then return end - local path = buf:name() - if not path then return end - local lang = pmacs.parse.language_for_path(path) - or pmacs.parse.language_from_shebang(buf) - if not lang then return end + -- Gate dispatch on `_has_language`: the chain above also resolves + -- languages with no grammar (python, javascript), and dispatching one + -- would raise "unknown language" (caught, but noise) — and an + -- extensionless script must never get a wrong-grammar parse tree. + local lang = resolve_active_language(buf) + if not lang or not pmacs.parse._has_language(lang) then return end pmacs.parse._dispatch(buf, lang) -- T M4.3: install the syntax-highlight overlay for this buffer. -- Idempotent --- repeated calls for the same buffer are a no-op @@ -175,10 +216,14 @@ local function reparse_active_buffer_after_edit() if not pmacs.parse._has_view(buf) then return end local pending = pmacs.parse._pending_edits(buf) if not pending or pending == 0 then return end - local path = buf:name() - if not path then return end - local lang = pmacs.parse.language_for_path(path) - or pmacs.parse.language_from_shebang(buf) + -- Reparse with the language pinned when the view was first attached --- + -- never re-resolve from the path or (mutable) shebang. The Rust side is + -- "first language wins"; re-sniffing a shebang the user just edited + -- would either raise "unknown language" (sh → python) or swap the parse + -- tree to a new grammar while the highlight overlay still holds the + -- original grammar's query (sh → lua). Language changes need a + -- close/reopen, exactly as they do for a renamed extension. + local lang = parse_lang_by_buffer[tostring(buf)] if not lang then return end pmacs.parse._dispatch(buf, lang) end diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 2d84aa1..9dacd83 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -5878,6 +5878,11 @@ fn m4_shebang_resolver_maps_interpreters() { ("#!/usr/bin/env bash\n", "bash"), ("#!/usr/bin/env python3\n", "python"), ("#!/usr/bin/env -S python3 -u\n", "python"), + // GNU-env options that consume an operand must not have the + // operand mistaken for the interpreter. + ("#!/usr/bin/env -u FOO python3\n", "python"), + ("#!/usr/bin/env -C /tmp python3\n", "python"), + ("#!/usr/bin/env -u FOO -C /tmp node\n", "javascript"), ("#!/usr/bin/node\n", "javascript"), ("#!/usr/bin/env lua\n", "lua"), ] { @@ -5957,17 +5962,132 @@ fn m4_shebang_does_not_override_extension() { )) .exec() .expect("open .py with a shell shebang"); - let lang: Option = s + // Both the LSP language *and* the grammar decision must respect the + // extension: python for LSP, and NO grammar parse view (python has no + // grammar) — not a bash tree installed from the `#!/bin/sh` line. + // `_has_view` is set synchronously by `_dispatch`, so no pump is + // needed; without the precedence fix the shebang would have dispatched + // bash and this would be true. + let (lang, has_view): (Option, bool) = s .lua_host .lua() - .load("return pmacs.lsp.active_buffer_language()") + .load( + "return pmacs.lsp.active_buffer_language(), + pmacs.parse._has_view(pmacs.window.buffer())", + ) .eval() - .expect("language"); + .expect("language + view"); assert_eq!( lang.as_deref(), Some("python"), - ".py extension wins over a #!/bin/sh shebang" + ".py extension wins over a #!/bin/sh shebang (LSP)" ); + assert!( + !has_view, + ".py file must not get a grammar parse view from a #!/bin/sh line" + ); +} + +/// Finding-1 gate: an extensionless `#!/usr/bin/env python3` script +/// resolves to python for LSP, but python has no grammar — syntax must +/// skip it *silently*. Without the `_has_language` gate, `_dispatch` +/// raises "unknown language: python" (caught by the after-load pcall and +/// reported through `pmacs.error`), which we assert does NOT happen. +#[test] +fn m4_shebang_extensionless_grammarless_language_is_silent() { + use pmacs::editor::EditorState; + let s = EditorState::new(); + let dir = tempfile::tempdir().expect("tempdir"); + let f = dir.path().join("generate"); // no extension + std::fs::write(&f, b"#!/usr/bin/env python3\nprint('hi')\n").expect("write"); + let f_disp = f.display(); + s.lua_host + .lua() + .load(format!( + "pmacs.lsp.config = {{}} + _G.__errs = {{}} + local real = pmacs.error + pmacs.error = function(m) table.insert(_G.__errs, m) end + pmacs.buffer.find_or_open('{f_disp}')" + )) + .exec() + .expect("open extensionless python script"); + let (lang, has_view, errs): (Option, bool, i64) = s + .lua_host + .lua() + .load( + "return pmacs.lsp.active_buffer_language(), + pmacs.parse._has_view(pmacs.window.buffer()), + #_G.__errs", + ) + .eval() + .expect("probe"); + assert_eq!(lang.as_deref(), Some("python"), "python resolves for LSP"); + assert!( + !has_view, + "no grammar parse view for a grammarless language" + ); + assert_eq!(errs, 0, "no 'unknown language' error reported"); +} + +/// Finding-2 pin: editing an open extensionless script's shebang must not +/// re-switch the parse grammar. A `#!/bin/sh` script attaches the bash +/// grammar; rewriting its shebang to lua and firing after-edit must keep +/// the bash tree (the pinned grammar) rather than swap in lua under the +/// stale highlight overlay — and must not error. +#[test] +fn m4_shebang_edit_keeps_pinned_grammar() { + use pmacs::editor::EditorState; + let mut s = EditorState::new(); + let dir = tempfile::tempdir().expect("tempdir"); + let hook = dir.path().join("deploy"); // no extension + std::fs::write(&hook, b"#!/bin/sh\necho one\n").expect("write"); + let hook_disp = hook.display(); + s.lua_host + .lua() + .load(format!( + "pmacs.lsp.config = {{}} + _G.__errs = {{}} + local real = pmacs.error + pmacs.error = function(m) table.insert(_G.__errs, m) end + pmacs.buffer.find_or_open('{hook_disp}')" + )) + .exec() + .expect("open extensionless shell script"); + pump_async(&mut s, |st| { + current_tree_language(st).as_deref() == Some("bash") + }); + + // Rewrite the first line to a lua shebang, then fire after-edit. + s.lua_host + .lua() + .load( + "local b = pmacs.window.buffer() + local text = b:slice(0, b:len()) + local first_len = (text:find('\\n', 1, true) or 1) - 1 + b:replace(0, first_len, '#!/usr/bin/env lua') + pmacs.hook.run('buffer.after-edit')", + ) + .exec() + .expect("rewrite shebang to lua"); + // Let the reparse settle (manual ticks: the tree stays bash with the + // pin, so a `pump_async` for a language *change* would time out). + for _ in 0..64 { + s.tick_async(); + std::thread::sleep(Duration::from_millis(2)); + } + assert_eq!( + current_tree_language(&s).as_deref(), + Some("bash"), + "editing the shebang must not re-switch the pinned grammar" + ); + let errs: i64 = s + .lua_host + .lua() + .load("return #_G.__errs") + .eval() + .expect("errs"); + assert_eq!(errs, 0, "reparse with the pinned grammar reports no error"); } /// Typing-perf: the default bundle coalesces full-document