[ousamabenyounes]

Summary

• rtk diff could report [ok] Files are identical with exit 0 for byte-different files whose only differences are hidden by str::lines(), such as CRLF-vs-LF or a trailing newline (rtk diff reports "Files are identical" (exit 0) for byte-different files #3469).
• Fix: pass the original file contents into the renderer and only use the identical branch when content1 == content2. If the line diff is empty but the file contents differ, rtk diff now prints an explicit whitespace/line-ending message and exits 1.
• This is rebased onto current develop and keeps the newer select_file_diff_output path, so the empty-diff message bypasses the tiny-output never_worse fallback that swallowed the reviewer-requested small-file case.
• Added Hydr0gen19's byte-identical CRLF guard test from fix(diff): compare files byte-accurately instead of by normalized lines #3471, adapted to this PR's current renderer signature with no fifth render_file_diff/renderer argument.

Test verification (RED -> GREEN)

New coverage:

• Unit tests for CRLF-vs-LF, trailing-newline-only, and byte-identical CRLF content.
• Run-level integration test for a small CRLF-vs-LF file pair through rtk diff, covering the review feedback path.

RED on unmodified origin/develop

assertion `left == right` failed: [ok] Files are identical

  left: Some(0)
 right: Some(1)
test result: FAILED. 0 passed; 1 failed

GREEN on this branch

test small_crlf_vs_lf_diff_prints_whitespace_message ... ok
test result: ok. 1 passed; 0 failed

Local CI replay:

• Baseline run-ci.sh on origin/develop: test-presence, fmt, clippy, and cargo test --all passed; unit suite 2929 passed; 0 failed; 8 ignored.
• Final run-ci.sh on this branch: test-presence, fmt, clippy, and cargo test --all passed; unit suite 2932 passed; 0 failed; 8 ignored; new integration test 1 passed.
• Coverage: cargo llvm-cov --all passed, and the Rust changed-line LCOV check found covered executable changed lines only (changed executable lines with coverage data: 80, PASS).

Test plan

• cargo fmt --all -- --check
• cargo clippy --all-targets
• cargo test --all
• cargo llvm-cov --lcov --output-path ... --all
• RED proof on unmodified origin/develop for the new run-level regression test

All PRs target the develop branch.

Fixes #3469

[KuSh]

Requesting changes on one point. The fix itself is right and I want it in — this is about it actually reaching the user.

Verified: diff.changes.is_empty() is equivalent to "the lines() vectors are element-wise equal", so the only byte differences that can survive that check are exactly \r\n-vs-\n and a trailing newline. The guard's message is therefore accurate, not a guess, and the exit code correctly becomes 1. cargo fmt --check, cargo clippy --all-targets and the 25 diff_cmd tests all pass on your head. I also ran the binary end-to-end against the cases in #3469.

The problem: for small files the message never gets printed. Details inline.

Worth knowing, since it explains why this is worth another round: I confirmed on 0.46.0 that the pre-existing behaviour already prints a raw dump for these inputs —

$ printf 'a\r\nb\r\nc\r\n' > x; printf 'a\nb\nc\n' > y
$ rtk diff x y; echo "exit=$?"
a
b
c

---
a
b
c
exit=0

— so a user comparing two small files today sees two visually identical blobs and exit 0. Your PR fixes the exit code half of that. If the message is also swallowed, they get two visually identical blobs and exit 1, which is better but still leaves them to work out why on their own. That's the gap worth closing while you're in here.

[KuSh]

Also, since @Hydr0gen19 has already approved it, could you include the test_render_byte_identical_exit_zero_with_crlf test from #3471 in your PR?

[Hydr0gen19]

Yes, please take it — copy it straight across, no attribution needed.

One adjustment when you do: the version on #3471 passes a fifth argument to render_file_diff (false), which belongs to an --ignore-whitespace flag that this PR rightly does not carry. Drop that argument and it compiles against your signature unchanged. Your guard sits inside the diff.changes.is_empty() branch rather than ahead of the split, but the case still reaches your identical path, so the assertions hold either way.

Why it is worth having next to your two: both of yours pin the "must differ" direction. This one pins the other side — two files that are both CRLF and byte-identical must still report identical and exit 0. A guard that over-triggers whenever it sees a CRLF would pass test_render_crlf_vs_lf_not_identical and test_render_trailing_newline_not_identical, and fail only this one.

[ousamabenyounes]

Thanks @KuSh and @Hydr0gen19. Rebased onto current develop and pushed f4325bc: the whitespace/EOL-only branch now uses the existing empty-diff output path to bypass the size guard, so small CRLF/LF files print the explanatory message and exit 1 instead of being swallowed into [ok] Files are identical.

I also included Hydr0gen19's byte-identical CRLF guard test from #3471, adapted to this PR's current renderer signature with no fifth render_file_diff/renderer argument.

Validation: the new run-level regression is RED on origin/develop ([ok] Files are identical, exit 0) and GREEN here. Local run-ci.sh baseline/final passed test-presence, fmt, clippy, and cargo test --all (2929 -> 2932 unit tests, plus the new integration test). cargo llvm-cov and the Rust changed-line coverage check passed.

Remote CI on f4325bc: test presence, fmt, doc review, clippy, Ubuntu/macOS/Windows tests, security scan, semgrep, and CLA passed. benchmark failed on the existing golangci-lint empty-output filter; the current develop run 33541678720 fails the same benchmark step with the same BENCHMARK FAILED: 1 filter(s) returned empty output summary.

[KuSh]

The message reaches the user now — clearing my earlier request for changes.

Rebasing onto develop was the right move and it solved this structurally rather than by special-casing. select_file_diff_output returns the rendered output unguarded when changes.is_empty(), which is exactly the whitespace branch, so the message survives at any file size instead of only above ~90 bytes:

$ rtk diff lf.txt crlf.txt          # 11 bytes each — previously printed a raw dump
lf.txt → crlf.txt
   files differ only in whitespace or line endings (no line-content change)
exit=1

And tests/diff_byte_accuracy_test.rs is the run()-level test I asked for — it spawns the real binary on the small fixture, which is precisely the case the unit tests were blind to.

Verified

cargo fmt --all --check and cargo clippy --all-targets clean, 3016 tests passing. Exit codes match /usr/bin/diff across the edge cases, and each branch is genuinely reachable:

	verdict	exit (rtk / GNU)
a\n vs a	whitespace message	1 / 1
CRLF vs LF	whitespace message	1 / 1
\n vs \r\n	whitespace message	1 / 1
a\r\n vs a\r\r\n	real diff	1 / 1
a\r vs a	real diff	1 / 1
byte-identical	identical	0 / 0

The last two matter: they confirm the message isn't over-claiming. str::lines() loses exactly two things — whether the final line ends in \n, and whether a \r preceded each \n — and both are line terminators, so "differ only in whitespace or line endings" is exact by construction rather than a best guess.

I also mutation-checked the new tests rather than trusting that they pass:

• byte check → line check (the original bug): both new unit tests fail.
• whitespace branch returning 0 instead of 1: both fail.
• removing the guard bypass so the message is swallowed again: small_crlf_vs_lf_diff_prints_whitespace_message fails with empty stdout.

That third one is the important one — the regression I flagged is now genuinely pinned, not passing by accident.

One small change pushed

I pushed 428af61 on top: render_diff was taking content1 and content2 only to compare them, which gave a rendering function five parameters and an input comparison to do. The caller already holds both strings, so it now passes content1 == content2 as a bytes_equal verdict. Behaviour is identical — same tests, same output, same exit codes, re-verified after the change. Revert it if you'd rather keep the contents in the signature.