Repository navigation
Conversation
github-actions
Bot
added
the
needs:compliance
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
github-actions
Bot
removed
the
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
ggbdpq
force-pushed
the
fix/edit-mixed-ending-conservative-match
branch
from
October 7, 2026 06:45
3a50cf9 to
dac0d1d
Compare
detectLineEnding flagged any file containing a single CRLF as a CRLF file, so oldString/newString written with LF endings - which is all the model can ever produce, since read strips \r - were rewritten to CRLF. In mixed-ending files the converted oldString no longer matched, and when the fallback matcher still found the region, the replacement landed with wrong line endings, silently corrupting the touched area. Detect CRLF only when every newline in the file is CRLF; mixed files now match as LF, consistent with the LF view read shows the model. Fully-CRLF and LF-only files behave exactly as before. Closes #45880 Generated-by: GLM-5.3-Flash (ZCode)
Author
|
Friendly ping — this has been open for a few days with green checks and no review feedback yet. Happy to rebase or adjust if anything needs changing. Thanks! |
Member
|
This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2. — from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #45880
Type of change
What does this PR do?
detectLineEndingpicked CRLF as soon as the file contained a single\r\n, and everything inoldString/newStringgot rewritten to CRLF. In a mixed-ending file that convertedoldStringno longer exists — the model can only ever produce LF strings, becausereadstrips\rbefore showing content. Worse, when the fallback matcher still located the region, the replacement landed with CRLF endings inside an LF area, silently corrupting those lines.The fix: a file only detects as CRLF when every newline in it is CRLF, so mixed files match as LF — consistent with the LF view
readshows. Fully-CRLF and LF-only files behave exactly as before.This is the conservative first step discussed in the issue. Editing the CRLF region of a mixed file still has open policy questions (preserve the file byte-for-byte vs normalize the touched region) that need a maintainer decision, so I left that out.
How did you verify your code works?
Added
matches LF regions of a mixed-ending filetopackages/opencode/test/tool/edit.test.ts. Before the fix the file came back with a stray\r\ninjected into the LF region (alpha\r\nbeta-updated\ngamma\ninstead ofalpha\nbeta-updated\ngamma\n); after the fix the LF region is edited and the CRLF line stays byte-identical.Full
packages/opencode/test/tool/suite: 342 pass / 0 fail (edit.test.ts is 30/30, including every existing CRLF-preservation case).tsgo --noEmitinpackages/opencodeis clean.Screenshots / recordings
Not a UI change.
Checklist