fix: strip CR from R output to prevent display corruption#56
Conversation
On Windows, R may produce CRLF line endings in error messages. When
printed to the terminal, the CR returns the cursor to the start of
the line, causing subsequent text to overwrite previous content,
resulting in garbled output like:
" の) : 予想外の invalid token です ( "{
Add normalize_crlf() helper function that converts CRLF to LF before
printing, following the same approach as radian.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
eitsupi
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes garbled error message display on Windows caused by CRLF line endings in R output. When R produces output with CRLF sequences on Windows, the carriage return (\r) causes the terminal cursor to return to the start of the line, overwriting previous content and creating garbled output.
Changes:
- Added
normalize_crlf()helper function that converts CRLF to LF using aCowfor efficiency - Applied CRLF normalization in the default output path of
r_write_console_exon Windows - Added comprehensive tests covering various edge cases including empty strings, mixed line endings, and standalone CR characters
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/arf-libr/src/sys.rs | Added normalize_crlf() function and applied it before printing R console output on Windows; includes comprehensive unit tests |
| CHANGELOG.md | Added entry in the Fixed section documenting the Windows CRLF fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/arf-libr/src/sys.rs
Outdated
| #[cfg(windows)] | ||
| let s = normalize_crlf(&s); |
There was a problem hiding this comment.
The CRLF normalization is only applied in the default output path (after line 372), but not in reprex mode (lines 336-366). On Windows, if R produces CRLF line endings in reprex mode, they won't be normalized before being processed character-by-character. This could lead to unexpected behavior where a CRLF sequence (\r\n) is treated as two separate characters - the \r will clear the line buffer and the \n will print it, which may not be the intended behavior.
Consider applying CRLF normalization before the reprex mode processing, or at the beginning of the function after the string is decoded.
crates/arf-libr/src/sys.rs
Outdated
| #[cfg(windows)] | ||
| let s = normalize_crlf(&s); |
There was a problem hiding this comment.
The CRLF normalization is not applied when a custom write console callback is set (lines 330-334). If consumers of this API use set_write_console_callback(), they will receive strings with CRLF line endings on Windows, which could cause display issues in their own output handling.
Consider applying CRLF normalization before passing the string to the callback, or document this behavior in the set_write_console_callback() function documentation so that callback implementers are aware they need to handle CRLF normalization themselves on Windows.
The issue was that R produces standalone CR characters (not CRLF) in
error messages, which causes the cursor to return to the start of the
line and overwrite previous content.
Changed from `replace("\r\n", "\n")` to `replace('\r', "")` to handle
both CRLF and standalone CR.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Only strip CR characters from error output (otype=1) to preserve progress bar functionality in normal output. Progress bars commonly use \r to overwrite the current line with updated progress. Co-Authored-By: Claude Opus 4.5 <[email protected]>
On Windows, reedline inserts CRLF for newlines when building multiline
input (via InsertNewline in line_buffer.rs). R doesn't accept CR in code
and treats it as an invalid token, causing "invalid token" errors for
any multiline input like `{ }` split across lines.
Reprex mode was unaffected because strip_reprex_output() uses
.lines().join("\n") which removes CR as a side effect.
This fix strips CR from code before passing it to R, matching how we
already handle CR in error output (PR #56).
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
normalize_crlf()helper function that converts CRLF to LF before printingTest plan
cargo test -p arf-libr test_normalize_crlfpasses🤖 Generated with Claude Code