Skip to content

fix(diff): rtk diff reports CRLF/LF-only differences as identical - #2643

Closed
amitmishra11 wants to merge 1 commit into
rtk-ai:developfrom
amitmishra11:fix/diff-crlf-lf-2627
Closed

amitmishra11 wants to merge 1 commit into
rtk-ai:developfrom
amitmishra11:fix/diff-crlf-lf-2627

Conversation

@amitmishra11

Copy link
Copy Markdown

Summary

Fixes #2627.

rtk diff <file1> <file2> reported two files as identical (exit 0) when they differed only in line-ending bytes (CRLF vs LF) — or, more generally, in anything str::lines() normalizes away (it strips \r along with \n, and treats a missing trailing newline as just "one fewer empty trailing line").

printf 'a\r\nb\r\nc\r\n' > x   # CRLF
printf 'a\nb\nc\n'       > y   # LF
rtk diff x y; echo "exit=$?"
# before: [ok] Files are identical / exit=0   (wrong — bytes differ, `diff` sees this)

Root cause

render_file_diff splits both files with content.lines() before comparing (src/cmds/git/diff_cmd.rs). str::lines() treats \r\n and \n as equivalent line terminators, so a CRLF file and an LF file with the same text collapse to identical line vectors, and compute_diff finds zero changes — the early-return then reports "identical" with no byte comparison.

Fix

In render_file_diff, when the line-based diff finds no changes, fall back to an explicit byte comparison (content1 == content2) before declaring the files identical:

  • Byte-equal → still "[ok] Files are identical", exit 0 (unchanged behavior).
  • Byte-different but line-identical → report the files differ, with a short description of the cause (describe_byte_only_difference: CRLF vs LF, trailing newline, or generic), exit 1 (diff convention).

Scope note

The issue also flags condense_unified_diff (this file) and compact_diff (src/cmds/git/git.rs) as sharing the same str::lines() pattern, but says those paths weren't independently verified to reproduce the bug. I scoped this PR to the confirmed, reproducible case (rtk diff <file> <file> / render_file_diff) rather than changing unverified code paths. Happy to follow up on those separately if confirmed.

Test plan

  • Added regression tests in src/cmds/git/diff_cmd.rs: CRLF-vs-LF (reproduces the exact issue), trailing-newline-only diff, and a same-CRLF-on-both-sides case to confirm true identical files still short-circuit correctly. Also unit tests for the new describe_byte_only_difference helper.
  • cargo test — could not run locally (no working MSVC/64-bit linker in my sandbox; see comment on fix: COPILOT_INSTRUCTIONS example drops kubectl get subcommand #2642 for details). The change is isolated to render_file_diff and a new pure helper function with no external dependencies; existing tests for that function (test_render_identical_files_exit_zero, test_render_modified_only_yaml_not_identical, etc.) are untouched and should still pass since the byte-equal fast path is preserved. Appreciate CI confirming.

str::lines() strips \r along with \n, so two files whose textual lines
match but whose raw bytes differ (CRLF vs LF terminators, or a missing
trailing newline) collapsed to an empty diff and were reported as
identical with exit 0, even though `diff`/git see them as different.

render_file_diff now falls back to a byte comparison when the line-based
diff finds no changes: true byte-equal files are still "identical" (exit
0), but a byte difference invisible to lines() is now surfaced with a
description (CRLF vs LF, trailing newline, or other) and exit 1, matching
diff-convention behavior.

Fixes rtk-ai#2627
@CLAassistant

CLAassistant commented Jun 26, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@KuSh

KuSh commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Closing as a duplicate of #3519, which adds the same byte-equality guard before the "identical" verdict.

The bug is confirmed live on 0.46.0:

$ printf 'a\r\nb\r\nc\r\n' > x; printf 'a\nb\nc\n' > y
$ rtk diff x y; echo "exit=$?"
[ok] Files are identical
exit=0

#3519 is the narrower change (57 lines, one file) and has a full green test matrix; this PR only ever ran check-target, which skipped, so it was never CI-verified.

That said, describe_byte_only_difference is the better message: #3519 says "whitespace or line endings" generically, where yours names the actual cause (line endings (CRLF vs LF) / trailing newline). I've noted it for #3519 to fold in with credit to you — a user told two files differ invisibly benefits from being told how.

@KuSh KuSh closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rtk diff reports "Files are identical" when files differ only in line endings (CRLF vs LF)

3 participants