fix(git): keep a repository root that contains a newline whole

`rev-parse --show-toplevel`'s output was parsed with `first_line`, which
takes `^[^\r\n]*`. A newline is a legal byte in a POSIX path, so a
repository rooted at `/tmp/a\nb` resolved to `/tmp/a` --- and every
command afterwards ran with a `-C` and a cwd naming a directory that
does not exist, turning a working repository into a wall of exit-128
failure rows.

Fixed with a SEPARATE helper, `strip_output_terminator`, used at that
one call site. `first_line` is deliberately left alone: its other three
callers --- the spawn-error text, the stderr detail, and the
`display_file` error string --- all feed the single-line status band,
where a multi-line message corrupts the row layout, so taking the first
line is exactly right for them. Folding the two together would fix one
caller and break three. Both functions now say at their definition which
kind of text they are for and why the other exists.

Exactly one trailing newline is stripped, with an optional preceding
carriage return, because that is what git emits as a terminator; a
second newline would be output rather than a terminator. The trailing
whitespace trim `first_line` also did is NOT carried over --- a path may
legally end in a space.

Witnessed end to end by `g6_14c`, not at the parser: the fixture really
creates `<tmp>/nl\nroot`, the real `git` resolves it, and the assertion
is on the cwd of the status spawn the module actually made, plus real
rows in the panel and a RET that opens the file the row names.
Mutation-verified --- restoring `first_line` there resolves the root to
`<tmp>/nl` and fails the test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
Levi Neuwirth 2026-08-09 16:18:41 +02:00
parent 3eca5e8f60
commit 842ec61f6f
No known key found for this signature in database
2 changed files with 90 additions and 1 deletions

View File

@ -290,11 +290,37 @@ pmacs.hook.add("process.after-tick", function()
end)
--- The first line of `text`, trimmed, or `""`.
---
--- For text bound for the ONE-LINE STATUS BAND, and only for that: a
--- spawn error, a stderr detail, an error string. A status message that
--- carried a newline would corrupt the row layout of whatever is
--- rendering it, so truncating to the first line is the right answer
--- there.
local function first_line(text)
local line = (text or ""):match("^[^\r\n]*") or ""
return (line:gsub("%s+$", ""))
end
--- `text` with git's final output terminator removed, and NOTHING else.
---
--- The counterpart to `first_line`, deliberately a SECOND function
--- rather than a change to it, because the two answer opposite
--- questions and each has callers that the other's answer would break.
--- This one is for COMMAND OUTPUT that must survive whole: a POSIX path
--- may legally contain a newline, so `git rev-parse --show-toplevel`
--- prints one for a repository rooted at `/tmp/a\nb`, and taking the
--- first line there truncates the root to `/tmp/a` --- after which every
--- command this module runs has a wrong or nonexistent cwd. `first_line`
--- has three other callers, all of them status-band text, and folding
--- the two together would fix this one and break those.
---
--- Exactly one trailing newline is stripped, with an optional preceding
--- carriage return, because that is what git emits. A second newline
--- would be output, not a terminator.
local function strip_output_terminator(text)
return ((text or ""):gsub("\r?\n$", ""))
end
--- A one-line description of why a git invocation failed.
local function failure_reason(res)
if res.spawn_error then
@ -752,7 +778,10 @@ function pmacs.git._deliver_root(request, res)
end
return
end
local root = first_line(res.stdout)
-- The WHOLE output, minus its terminator --- never the first line. A
-- repository root may contain a newline, and truncating one here would
-- point every command that follows at a directory that does not exist.
local root = strip_output_terminator(res.stdout)
if root == "" then
pmacs.editor.set_status("git: rev-parse returned no worktree root")
return

View File

@ -1528,6 +1528,66 @@ fn g6_14_the_root_rule_works_where_project_kind_is_not_git() {
);
}
/// A repository root containing a **newline** resolves **whole**, and
/// the status command really runs there.
///
/// A newline is a legal byte in a POSIX path — the fixture below builds
/// one and `git rev-parse --show-toplevel` prints it, terminator and
/// all — so parsing that output with a first-line match truncates
/// `/tmp/…/nl\nroot` to `/tmp/…/nl`, and every command this module runs
/// afterwards gets a `-C` and a cwd naming a directory that does not
/// exist. The right answer is to strip git's final terminator and
/// nothing else.
///
/// End to end, not at the parser: the directory really is created, the
/// real `git` really resolves it, and the assertion is on the cwd of the
/// spawn the module actually made. `_last_spawn` is the status
/// invocation here, since `rev-parse` runs first and carries no cwd of
/// its own.
///
/// It rides beside the one-line-status rule rather than replacing it:
/// the helper this uses is deliberately **separate** from `first_line`,
/// whose other three callers all feed the single-line status band and
/// would be corrupted by a multi-line message.
#[test]
fn g6_14c_a_root_containing_a_newline_is_not_truncated() {
let (_dir, base) = tempdir();
let root = base.join("nl\nroot");
std::fs::create_dir_all(&root).expect("a newline is a legal POSIX path byte");
mixed_repo(&root);
let mut s = editor();
open_panel(&mut s, &root, "staged.txt");
let cwd: String = eval(&s, "return pmacs.git._last_spawn.cwd");
assert_eq!(
cwd,
root.display().to_string(),
"the resolved root must be the WHOLE path, newline included"
);
let text = panel_text(&s);
assert!(
!text.contains("exited with code"),
"…so the status command ran somewhere that exists: {text}"
);
assert!(
text.contains("staged.txt"),
"…and produced real rows: {text}"
);
// And the root is usable for the gestures built on it: a
// repository-relative row path resolves against it to a real file.
seat_on(&mut s, "unstaged.txt");
press(&mut s, KeyCode::Enter);
assert_eq!(
active_name(&s),
root.join("unstaged.txt").display().to_string(),
"RET resolves against the untruncated root; status was {:?}",
status(&s)
);
}
/// A directory outside any repository reports it, rather than opening
/// an empty panel or saying nothing.
#[test]