Fixing Json character offsets via wrapping the underlying character buffer"#2643
Draft
jurgenvinju wants to merge 18 commits intomainfrom
Draft
Fixing Json character offsets via wrapping the underlying character buffer"#2643jurgenvinju wants to merge 18 commits intomainfrom
jurgenvinju wants to merge 18 commits intomainfrom
Conversation
…he JsonReader. idea via @DavyLandman
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2643 +/- ##
=======================================
Coverage 46% 46%
+ Complexity 6673 6672 -1
=======================================
Files 795 795
Lines 65878 65895 +17
Branches 9870 9872 +2
=======================================
+ Hits 30682 30713 +31
+ Misses 32825 32806 -19
- Partials 2371 2376 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DavyLandman
reviewed
Feb 11, 2026
Comment on lines
1133
to
1134
| // we've read until here before we were reset to 0. | ||
| offset += limit; |
Member
There was a problem hiding this comment.
this is not correct, we should do offset += limit - off and later also take limit and the off in the account.
I think it should be (if we want to use these 2 variables):
offset += limit - off;
limit = off + in.read(cbuf, off, len);
readCount++;
return limit - off;…oid NPEs and other crashes
jurgenvinju
force-pushed
the
fix/json-offsets
branch
from
8614a3d to
d8dec4d
Compare
…n positions on objects
|
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
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.
This continues where #2638 left off.
ParseError(locationTop or |unknown:///|, line, column, cause, path)instead ofParseError(locWithOffsetLineColumn, cause, path)trackOrigins=falsefor the public Rascal API.trackOriginshas no effect. It is alwaysoff.TODO
OriginTrackingReader(nested class inJsonValueReader), we require a test which triggers non-complete-buffer fill without being at EOF. The read must be asked to fill the complete buffer, but it should succeed with a smaller amount of characters read. Then the next call to theOriginTrackingReaderwill haveoff!=0which is the path we haven't tested yet.readanymore, because we've already accepted the input. And so this case does not triggeroff!=0This API still streams pretty quickly. Could not measure a degredation in speed on smaller files like we have in the tests. On very larger files sometimes it's 1% slower and sometimes 1% faster, so that seems to be noise.
This is for another PR