fix(core): resolve Windows line ending and path separation bugs across CLI #21068
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the CLI's cross-platform compatibility, particularly for Windows environments. It resolves critical bugs related to how the application processes line endings and path separators, ensuring consistent and correct behavior in diff rendering, code highlighting, and file utility operations across different operating systems. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses cross-platform compatibility issues on Windows by replacing platform-specific line splitting and path manipulation with platform-agnostic methods. The use of split(/\r?\n/) for line endings and path.basename() for extracting filenames is appropriate and resolves the described bugs. I have one concern regarding the removal of an ESLint disable comment, which could potentially cause CI failures.
.claude/worktrees/nervous-sutherland
Outdated
There was a problem hiding this comment.
Yes remove it, it was added by mistake.
There was a problem hiding this comment.
Looks like this is still not removed.
There was a problem hiding this comment.
Done! The .claude/worktrees/nervous-sutherland folder has been removed. Sorry for the delay on this!
456dfd4 to
67d0675
Compare
|
Thanks for fixing! |
Summary for #20967
Fixes Windows cross-platform compatibility bugs where
\r\n(CRLF) line endingscaused rendering artifacts and incorrect line counting, and where manual path
splitting by
/broke filename extraction on Windows (which uses\).Details
Root Cause:
.split('\n')to split text into lines.On Windows, line endings are
\r\n, so splitting by\nalone leaves atrailing
\ron each line, causing rendering artifacts inDiffRendererandCodeColorizer, and incorrect line counts infileUtils.ts.filePath.split('/').pop()to extractfilenames. On Windows, paths use
\as the separator, so this fails toextract the correct filename.
Fix:
.split('\n')with.split(/\r?\n/)in 4 locations across 3 files.The regex handles both Unix (
\n) and Windows (\r\n) line endings.filePath.split('/').pop()withpath.basename(filePath)which isthe idiomatic, platform-aware Node.js API for extracting filenames.
Impact: The CLI now correctly handles Windows line endings and paths in diff
rendering, code syntax highlighting, file content processing, and tests.
Files Changed
packages/cli/src/ui/components/messages/DiffRenderer.tsx.split('\n')→.split(/\r?\n/)packages/cli/src/ui/utils/CodeColorizer.tsx.split('\n')→.split(/\r?\n/)(2 locations)packages/core/src/utils/fileUtils.ts.split('\n')→.split(/\r?\n/)packages/core/src/tools/read-many-files.test.tsfilePath.split('/').pop()→path.basename(filePath)Related Issues
Fixes #20967
How to Validate
\r\n) andverify:
DiffRenderershows clean output without trailing\rcharacters
CodeColorizerrenders correctlyfileUtils.tsreturns accurate countsread-many-filestests on Windows — filenameextraction should work correctly with backslash paths
npm test -w @google/gemini-cli-core -- src/tools/read-many-files.test.ts—all 31 tests pass
Pre-Merge Checklist
is in test file itself)
npm runnpxnpm runnpxnpm runnpx