fix(tui): strip CRLF artifacts in diff view#9517
fix(tui): strip CRLF artifacts in diff view#9517conqueror wants to merge 1 commit intoopenai:mainfrom
Conversation
|
Thanks for the PR. I don't know that this is the correct fix for this bug. This is papering over the underlying problem. I'll discuss with my colleagues and see what they think. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Orb Code Review (powered by GLM-4.7 on Orb Cloud) SummaryThis PR fixes a display issue in the TUI diff view where CRLF (Carriage Return + Line Feed) line endings from Windows-style files would cause rendering artifacts. The fix strips trailing carriage returns ( ArchitectureThe fix:
// Before:
let mut remaining_text: &str = text;
// After:
let mut remaining_text: &str = text.trim_end_matches('\r');
#[test]
fn trims_crlf_in_diff_output() {
let original = "one\r\ntwo\r\n";
let modified = "one\r\nTWO\r\n";
let patch = diffy::create_patch(original, modified).to_string();
// Test update, add, and delete operations
changes.insert(PathBuf::from("crlf_update.txt"), FileChange::Update { ... });
changes.insert(PathBuf::from("crlf_add.txt"), FileChange::Add { content: "alpha\r\nbeta\r\n".to_string() });
changes.insert(PathBuf::from("crlf_delete.txt"), FileChange::Delete { content: "gamma\r\ndelta\r\n".to_string() });
let lines = diff_summary_for_tests(&changes);
let text = lines.iter().map(|l| l.spans.iter().map(|s| s.content.as_ref()).collect::<String>()).collect::<Vec<_>>().join("\n");
assert_eq!(text.contains('\r'), false);
}The problem: The solution: IssuesNo issues found. This is a well-executed bug fix that:
Cross-file ImpactAffected components:
Breaking changes: None. This is a display bug fix. Behavior changes:
Cross-platform impact:
Assessment✅ Approve Simple but effective fix for a real cross-platform display issue: Strengths:
User benefit: |
Summary
Testing
just fmtcargo test -p codex-tuicargo test -p codex-tui2just fix -p codex-tuiFixes #9455