From 7d3f38936dbe6baffbb4df12f133450246338412 Mon Sep 17 00:00:00 2001 From: Nagendra Mohan Date: Mon, 24 Aug 2026 23:09:52 +0530 Subject: [PATCH 1/2] diff: place -y gutter marker at the middle of the gutter In side-by-side output the gutter marker was written wherever process_half_line stopped padding the left half (one column past sdiff_half_width), not at separator_pos (the middle of the gutter). Those agree only when the gutter is 3 or 4 columns wide, so the default --width=130 --tabsize=8 (gutter 3) looked right while other widths/tab sizes drifted (e.g. --width=40 placed '<' at column 17 instead of 19). Pad from half_width + 1 up to separator_pos before writing the marker so it lands at the gutter middle, matching GNU sdiff. format_tabs_and_spaces is a no-op when already at/past separator_pos, so narrow gutters are unaffected. Add a regression test at --width=40. Fixes #269 --- src/side_diff.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/side_diff.rs b/src/side_diff.rs index 56953d25..bcf57c97 100644 --- a/src/side_diff.rs +++ b/src/side_diff.rs @@ -285,6 +285,11 @@ fn push_output( // the diff always want to put all tabs possible in the usable are, // even in the middle space between the gutters if possible. + // `process_half_line` left the cursor one column past the half width. The gutter marker + // belongs at the middle of the gutter (`separator_pos`), which is only `half_width + 1` + // when the gutter is 3 or 4 columns wide; pad up to it for wider gutters so the marker + // doesn't drift (see #269). `format_tabs_and_spaces` is a no-op when already at/past it. + format_tabs_and_spaces(half_width + 1, separator_pos, config, output)?; output.write_all(&[symbol])?; if !right_ln.is_empty() { format_tabs_and_spaces(separator_pos + 1, column_two_offset, config, output)?; @@ -1003,6 +1008,29 @@ mod tests { assert_eq!(contains_string(&output, "equal"), 2) } + // Regression test for #269: the `-y` gutter marker must sit at the middle of the gutter + // (separator_pos) regardless of gutter width. At the default width (gutter 3) the marker + // was already correct; at other widths it drifted. Here width=40 yields a wider gutter, so + // the marker must land at column 19 (matching GNU), not the old `half_width + 1`. + #[test] + fn test_gutter_marker_column_wide_gutter() { + let params = Params { + tabsize: 8, + expand_tabs: true, + width: 40, + ..Default::default() + }; + let mut output = vec![]; + diff(b"aa\n", b"", &mut output, ¶ms); + let text = String::from_utf8(output).unwrap(); + let line = text.lines().next().unwrap(); + assert_eq!( + line.find('<'), + Some(19), + "gutter marker '<' should be at column 19 for --width=40, got: {line:?}" + ); + } + #[test] fn test_different_lines() { let params = generate_params(); From ab9f8d931f6cccee2e506f9f0ea8b9b5fbe0325a Mon Sep 17 00:00:00 2001 From: nagendramohan Date: Tue, 29 Sep 2026 11:14:36 +0530 Subject: [PATCH 2/2] =?UTF-8?q?diff:=20address=20review=20=E2=80=94=20shor?= =?UTF-8?q?ten=20comments=20and=20add=20-y=20width=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shorten the two long comments per review, and add an integration test in tests/integration.rs exercising `-y --width=40` in both the default (expand_tabs: false, tab-padding path) and --expand-tabs modes, asserting the gutter marker lands at the middle of the gutter. --- src/side_diff.rs | 10 ++------- tests/integration.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/side_diff.rs b/src/side_diff.rs index bcf57c97..1128e2e7 100644 --- a/src/side_diff.rs +++ b/src/side_diff.rs @@ -285,10 +285,7 @@ fn push_output( // the diff always want to put all tabs possible in the usable are, // even in the middle space between the gutters if possible. - // `process_half_line` left the cursor one column past the half width. The gutter marker - // belongs at the middle of the gutter (`separator_pos`), which is only `half_width + 1` - // when the gutter is 3 or 4 columns wide; pad up to it for wider gutters so the marker - // doesn't drift (see #269). `format_tabs_and_spaces` is a no-op when already at/past it. + // Pad to the middle of the gutter so the marker doesn't drift on wider gutters (#269). format_tabs_and_spaces(half_width + 1, separator_pos, config, output)?; output.write_all(&[symbol])?; if !right_ln.is_empty() { @@ -1008,10 +1005,7 @@ mod tests { assert_eq!(contains_string(&output, "equal"), 2) } - // Regression test for #269: the `-y` gutter marker must sit at the middle of the gutter - // (separator_pos) regardless of gutter width. At the default width (gutter 3) the marker - // was already correct; at other widths it drifted. Here width=40 yields a wider gutter, so - // the marker must land at column 19 (matching GNU), not the old `half_width + 1`. + // Regression for #269: the `-y` marker must sit at the gutter middle at any width. #[test] fn test_gutter_marker_column_wide_gutter() { let params = Params { diff --git a/tests/integration.rs b/tests/integration.rs index 12aabb84..99cce275 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -343,6 +343,54 @@ mod diff { Ok(()) } + + // Visual (tab-aware) column of the `-y` gutter marker on the first output line. + fn side_marker_visual_column(output: &[u8], tabsize: usize) -> Option { + let text = String::from_utf8_lossy(output); + let line = text.lines().next()?; + let mut col = 0; + for c in line.chars() { + match c { + '<' | '|' | '>' => return Some(col), + '\t' => col += tabsize - (col % tabsize), + _ => col += 1, + } + } + None + } + + // Regression for #269: the `-y` gutter marker must sit at the middle of the gutter. + #[test] + fn sdiff_gutter_marker_column() -> Result<(), Box> { + let mut file1 = NamedTempFile::new()?; + file1.write_all("aa\n".as_bytes())?; + let mut file2 = NamedTempFile::new()?; + file2.write_all("bb\n".as_bytes())?; + + // --expand-tabs: padding is spaces, marker lands at visual column 19. + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-y") + .arg("--width=40") + .arg("--expand-tabs") + .arg(file1.path()) + .arg(file2.path()); + let output = cmd.output().unwrap().stdout; + assert_eq!(side_marker_visual_column(&output, 8), Some(19)); + + // Default (expand_tabs: false): padding goes through the tab path, but the + // marker must still land at the same visual column. + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("diff") + .arg("-y") + .arg("--width=40") + .arg(file1.path()) + .arg(file2.path()); + let output = cmd.output().unwrap().stdout; + assert_eq!(side_marker_visual_column(&output, 8), Some(19)); + + Ok(()) + } } mod cmp {