diff --git a/builtin/runtime/git.lua b/builtin/runtime/git.lua index d322ba6..bfac410 100644 --- a/builtin/runtime/git.lua +++ b/builtin/runtime/git.lua @@ -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 diff --git a/tests/git_status_stage1_acceptance.rs b/tests/git_status_stage1_acceptance.rs index 4126628..1dbb5a6 100644 --- a/tests/git_status_stage1_acceptance.rs +++ b/tests/git_status_stage1_acceptance.rs @@ -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]