From fda1d27be50a4d9472996422f3532fafb7e3f39c Mon Sep 17 00:00:00 2001 From: Luca Vitale Date: Fri, 7 Aug 2026 17:57:06 +0200 Subject: [PATCH] fix(diff): compare files byte-accurately instead of by normalized lines `rtk diff a b` split both files with `str::lines()`, which drops the line terminator. CRLF vs LF and a missing final newline therefore produced an empty change list, so byte-different files were reported as "[ok] Files are identical" and exited 0. Since the `diff` rewrite rule routes plain `diff a b` through rtk, this made rtk unusable as an equality oracle in scripts and agent workflows. Byte equality is now the only path to exit 0. When the contents differ but every line matches, the terminators are the only possible cause and the output says so instead of claiming the files are identical. `--ignore-whitespace` keeps the lenient comparison available: it trims trailing whitespace and ignores line endings and the final newline. --- docs/usage/FEATURES.md | 8 +++ src/cmds/git/diff_cmd.rs | 148 +++++++++++++++++++++++++++++++++++++-- src/main.rs | 11 ++- 3 files changed, 158 insertions(+), 9 deletions(-) diff --git a/docs/usage/FEATURES.md b/docs/usage/FEATURES.md index 8afaa62022..b2d34589ea 100644 --- a/docs/usage/FEATURES.md +++ b/docs/usage/FEATURES.md @@ -246,6 +246,14 @@ rtk diff rtk diff # Stdin comme second fichier ``` +**Options :** + +| Option | Defaut | Description | +|--------|--------|-------------| +| `--ignore-whitespace` | non | Ignore les espaces en fin de ligne, les fins de ligne (CRLF/LF) et le saut de ligne final | + +La comparaison se fait au niveau des octets : deux fichiers ne sont declares identiques (code de sortie 0) que si leur contenu est strictement egal. Toute difference renvoie le code 1. + --- ### `rtk wc` -- Comptage compact diff --git a/src/cmds/git/diff_cmd.rs b/src/cmds/git/diff_cmd.rs index f40c857f41..596a4a09a8 100644 --- a/src/cmds/git/diff_cmd.rs +++ b/src/cmds/git/diff_cmd.rs @@ -8,7 +8,7 @@ use std::path::Path; /// Ultra-condensed diff - only changed lines, no context. /// Returns the diff-convention exit code: 0 if identical, 1 if files differ. -pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result { +pub fn run(file1: &Path, file2: &Path, ignore_whitespace: bool, verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); if verbose > 0 { @@ -19,7 +19,7 @@ pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result { let content2 = fs::read_to_string(file2)?; let raw = format!("{}\n---\n{}", content1, content2); - let (rtk, exit_code) = render_file_diff(file1, file2, &content1, &content2); + let (rtk, exit_code) = render_file_diff(file1, file2, &content1, &content2, ignore_whitespace); let shown = never_worse(&raw, &rtk); print!("{}", shown); @@ -32,15 +32,45 @@ pub fn run(file1: &Path, file2: &Path, verbose: u8) -> Result { Ok(exit_code) } +/// Splits into comparable lines. Line terminators are always dropped, so byte +/// equality is checked separately by the caller. +fn split_lines(content: &str, ignore_whitespace: bool) -> Vec<&str> { + if ignore_whitespace { + content.lines().map(str::trim_end).collect() + } else { + content.lines().collect() + } +} + /// Renders the condensed file comparison and returns it with the /// diff-convention exit code (0 = identical, 1 = differences found). -fn render_file_diff(file1: &Path, file2: &Path, content1: &str, content2: &str) -> (String, i32) { - let lines1: Vec<&str> = content1.lines().collect(); - let lines2: Vec<&str> = content2.lines().collect(); +fn render_file_diff( + file1: &Path, + file2: &Path, + content1: &str, + content2: &str, + ignore_whitespace: bool, +) -> (String, i32) { + if content1 == content2 { + return ("[ok] Files are identical\n".to_string(), 0); + } + + let lines1 = split_lines(content1, ignore_whitespace); + let lines2 = split_lines(content2, ignore_whitespace); let diff = compute_diff(&lines1, &lines2); if diff.changes.is_empty() { - return ("[ok] Files are identical\n".to_string(), 0); + if ignore_whitespace { + return ("[ok] Files are identical (ignoring whitespace)\n".to_string(), 0); + } + // Bytes differ but every line matches: only the line terminators can + // be responsible, and those never survive into the change list. + let rtk = format!( + "{} → {}\n identical line content; files differ in line endings or the final newline\n", + file1.display(), + file2.display() + ); + return (rtk, 1); } let mut rtk = String::new(); @@ -320,6 +350,7 @@ mod tests { Path::new("two.yaml"), "a: 1\n", "a: 2\n", + false, ); assert!( !out.contains("identical"), @@ -339,6 +370,7 @@ mod tests { Path::new("j2.json"), "{\"a\": 1}\n", "{\"a\": 2}\n", + false, ); assert!( !out.contains("identical"), @@ -355,6 +387,7 @@ mod tests { Path::new("b.yaml"), "a: 1\nb: 2\n", "a: 1\nb: 2\n", + false, ); assert!(out.contains("[ok] Files are identical")); assert_eq!(code, 0); @@ -362,11 +395,112 @@ mod tests { #[test] fn test_render_added_removed_exit_one() { - let (out, code) = render_file_diff(Path::new("t1.txt"), Path::new("t2.txt"), "x\n", "y\n"); + let (out, code) = render_file_diff( + Path::new("t1.txt"), + Path::new("t2.txt"), + "x\n", + "y\n", + false, + ); assert!(out.contains("+1 added, -1 removed")); assert_eq!(code, 1); } + // --- byte accuracy (issue #3469) --- + + #[test] + fn test_render_crlf_vs_lf_not_identical() { + let (out, code) = render_file_diff( + Path::new("crlf.txt"), + Path::new("lf.txt"), + "a\r\nb\r\n", + "a\nb\n", + false, + ); + assert!( + !out.contains("[ok] Files are identical"), + "CRLF vs LF reported as identical:\n{}", + out + ); + assert!(out.contains("line endings")); + assert_eq!(code, 1, "byte-different files must exit 1"); + } + + #[test] + fn test_render_missing_final_newline_not_identical() { + let (out, code) = render_file_diff( + Path::new("nl.txt"), + Path::new("no_nl.txt"), + "a\nb\n", + "a\nb", + false, + ); + assert!( + !out.contains("[ok] Files are identical"), + "missing final newline reported as identical:\n{}", + out + ); + assert_eq!(code, 1); + } + + #[test] + fn test_render_trailing_space_not_identical() { + let (out, code) = render_file_diff( + Path::new("sp.txt"), + Path::new("no_sp.txt"), + "a \n", + "a\n", + false, + ); + assert!(!out.contains("[ok] Files are identical")); + assert_eq!(code, 1); + } + + #[test] + fn test_render_ignore_whitespace_matches_trailing_space() { + let (out, code) = render_file_diff( + Path::new("sp.txt"), + Path::new("no_sp.txt"), + "a \nb\t\n", + "a\nb\n", + true, + ); + assert!(out.contains("[ok] Files are identical (ignoring whitespace)")); + assert_eq!(code, 0); + } + + #[test] + fn test_render_ignore_whitespace_still_reports_content_changes() { + let (out, code) = render_file_diff( + Path::new("one.yaml"), + Path::new("two.yaml"), + "a: 1 \n", + "a: 2\n", + true, + ); + assert!(!out.contains("identical")); + assert_eq!(code, 1); + } + + #[test] + fn test_render_byte_identical_exit_zero_with_crlf() { + let (out, code) = render_file_diff( + Path::new("a.txt"), + Path::new("b.txt"), + "a\r\nb\r\n", + "a\r\nb\r\n", + false, + ); + assert!(out.contains("[ok] Files are identical")); + assert_eq!(code, 0); + } + + #[test] + fn test_split_lines_ignore_whitespace_trims_line_ends() { + assert_eq!(split_lines("a \nb\t\n", true), vec!["a", "b"]); + assert_eq!(split_lines("a \nb\t\n", false), vec!["a ", "b\t"]); + } + // --- condense_unified_diff --- #[test] diff --git a/src/main.rs b/src/main.rs index b29cf0769b..ebe7766855 100644 --- a/src/main.rs +++ b/src/main.rs @@ -272,6 +272,9 @@ enum Commands { file1: PathBuf, /// Second file (optional if stdin) file2: Option, + /// Treat trailing whitespace, line endings and the final newline as equal + #[arg(long)] + ignore_whitespace: bool, }, /// Filter and deduplicate log output @@ -1884,9 +1887,13 @@ fn run_cli() -> Result { 0 } - Commands::Diff { file1, file2 } => { + Commands::Diff { + file1, + file2, + ignore_whitespace, + } => { if let Some(f2) = file2 { - diff_cmd::run(&file1, &f2, cli.verbose)? + diff_cmd::run(&file1, &f2, ignore_whitespace, cli.verbose)? } else { diff_cmd::run_stdin(cli.verbose)?; 0