fix(read): correct file offset calculation for CRLF line endings #4349
+39
−4
Conversation
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
Fixes assertion failure in getInputFileOffsetForLine when processing files with CRLF line endings. The issue occurred because crAdjustment included the current line's CRLF conversion, but the stored position was captured before processing that conversion. The fix uses the previous line's crAdjustment value instead, ensuring only adjustments that occurred before the stored position are subtracted. This resolves the timing mismatch that caused negative offset calculations. Fixes universal-ctags#4343
…ine offset functions The original fix for issue 4343 corrected getInputFileOffsetForLine to prevent negative calculations by using the previous line's crAdjustment instead of the current line's. However, this created an inconsistency with compoundPosForOffset (used in binary search), which still used the current line's crAdjustment. This inconsistency caused assertion failures in makePromiseForAreaSpecifiedWithOffsets where startOffset >= startLineOffset was expected to hold true, but the two functions returned different offset values for the same logical position. Updated compoundPosForOffset to use the same timing-corrected logic as getInputFileOffsetForLine, ensuring both functions account for the timing mismatch where positions are stored before CRLF processing but crAdjustment includes the current line's CRLF conversion. Fixes CI failure in parser-xml.r/dos-eol test case while maintaining the original fix for negative offset calculations in CRLF files.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4349 +/- ##
=======================================
Coverage 85.87% 85.87%
=======================================
Files 252 252
Lines 62597 62614 +17
=======================================
+ Hits 53755 53772 +17
Misses 8842 8842 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@masatake any updates on this? |
1 similar comment
Contributor
Author
|
@masatake any updates on this? |
Member
|
Please wait for awhile. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes assertion failure in getInputFileOffsetForLine when processing files with CRLF line endings. The issue occurred because
crAdjustmentincluded the current line's CRLF conversion, but the stored position was captured before processing that conversion.The fix uses the previous line's crAdjustment value instead, ensuring only adjustments that occurred before the stored position are subtracted. This resolves the timing mismatch that caused negative offset calculations.
Fixes #4343