Skip to content

Commit d536891

Browse files
strickddkylehgc
authored andcommitted
fix(git): keep the trailing newline on filtered 'git status' output
`filter_status_with_args` joins kept lines with \n and has no terminal newline; `never_worse` may instead return git's raw stdout, which does. `print!` on either is wrong in one of the two cases, so the arg-carrying status path (--short / -s / --porcelain) dropped the final newline. The last entry then glues to whatever prints next, and — worse than cosmetic — any pipeline that counts lines undercounts by one: git status --porcelain | cat -A -> ?? handoff/$ (1 line) rtk git status --porcelain | cat -A -> ?? handoff/ (0 lines) On a tree with a single change, `rtk git status --porcelain | wc -l` therefore reports 0 — a false 'clean' on the exact question the command is usually asked. Anything gating a commit/no-commit decision on piped porcelain output takes the wrong branch. Normalize to exactly one trailing newline via a pure, unit-tested `with_trailing_newline`. Empty output stays empty: --porcelain on a clean tree correctly prints nothing.
1 parent 01e7ac1 commit d536891

1 file changed

Lines changed: 50 additions & 1 deletion

File tree

‎src/cmds/git/git.rs‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,25 @@ fn filter_status_with_args(output: &str) -> String {
14691469
}
14701470
}
14711471

1472+
/// Ensure filtered output ends with exactly one trailing newline, matching git.
1473+
///
1474+
/// `filter_status_with_args` joins its kept lines with `\n` and therefore has no
1475+
/// terminal newline, while `never_worse` may hand back git's raw stdout, which
1476+
/// does. Printing either verbatim is wrong in one of the two cases: the joined
1477+
/// form glues its last entry to whatever prints next, so
1478+
/// `rtk git status --porcelain | wc -l` undercounts by one — reporting `0`, i.e.
1479+
/// "clean", on a tree with a single change.
1480+
///
1481+
/// Empty output is left empty: `--porcelain` on a clean tree prints nothing at
1482+
/// all, and a lone newline there would be a different fidelity bug.
1483+
fn with_trailing_newline(output: &str) -> String {
1484+
if output.is_empty() || output.ends_with('\n') {
1485+
output.to_string()
1486+
} else {
1487+
format!("{}\n", output)
1488+
}
1489+
}
1490+
14721491
fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result<i32> {
14731492
let timer = tracking::TimedExecution::start();
14741493

@@ -1527,7 +1546,7 @@ fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result<i3
15271546

15281547
// Apply minimal filtering: strip ANSI, remove hints, empty lines
15291548
let filtered = filter_status_with_args(&result.stdout);
1530-
let filtered = never_worse(&result.stdout, &filtered).to_string();
1549+
let filtered = with_trailing_newline(never_worse(&result.stdout, &filtered));
15311550
print!("{}", filtered);
15321551

15331552
timer.track(
@@ -2951,6 +2970,36 @@ mod tests {
29512970
assert_eq!(args, vec!["status", "--porcelain", "-b"]);
29522971
}
29532972

2973+
#[test]
2974+
fn with_trailing_newline_appends_when_missing() {
2975+
assert_eq!(
2976+
with_trailing_newline(" M tracked.txt\n?? zzz-last.txt"),
2977+
" M tracked.txt\n?? zzz-last.txt\n"
2978+
);
2979+
}
2980+
2981+
#[test]
2982+
fn with_trailing_newline_leaves_existing_newline_alone() {
2983+
assert_eq!(with_trailing_newline(" M tracked.txt\n"), " M tracked.txt\n");
2984+
}
2985+
2986+
#[test]
2987+
fn with_trailing_newline_keeps_empty_output_empty() {
2988+
// `--porcelain` on a clean tree prints nothing; a lone newline would be
2989+
// its own fidelity bug.
2990+
assert_eq!(with_trailing_newline(""), "");
2991+
}
2992+
2993+
#[test]
2994+
fn filtered_status_line_count_matches_git() {
2995+
// The regression this guards: `rtk git status --porcelain | wc -l`
2996+
// reported 0 on a one-file dirty tree, i.e. a false "clean".
2997+
let raw = "?? handoff/\n";
2998+
let filtered = with_trailing_newline(&filter_status_with_args(raw));
2999+
assert_eq!(filtered.lines().count(), raw.lines().count());
3000+
assert_eq!(filtered, raw);
3001+
}
3002+
29543003
#[test]
29553004
fn test_uses_compact_status_path_for_branch_and_short_flags() {
29563005
assert!(uses_compact_status_path(&["-b".to_string()]));

0 commit comments

Comments
 (0)