Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix diagnostics covering trailing text #317

Merged
merged 1 commit into from
Jun 26, 2023
Merged

Conversation

c42f
Copy link
Member

@c42f c42f commented Jun 25, 2023

Diagnostics may sometimes cover trailing text which wasn't consumed. For example, parseatom(")") doesn't consume the errant trailing ')', but the diagnostic refers to this character. In this case, SourceFile needs to cover the location of the diagnostic in addition to any text which is consumed.

As part of this, mark sourcetext(::ParseStream) as deprecated, in favour of constructing a SourceFile to wrap things up more neatly.

Also fix a bug in highlight() for empty SourceFiles.

Fixes #314 and downstream issue JuliaLang/julia#50245

@codecov
Copy link

codecov bot commented Jun 25, 2023

Codecov Report

Merging #317 (fcdd8cf) into main (8731bab) will decrease coverage by 0.24%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##             main     #317      +/-   ##
==========================================
- Coverage   96.80%   96.56%   -0.24%     
==========================================
  Files          14       14              
  Lines        4125     4138      +13     
==========================================
+ Hits         3993     3996       +3     
- Misses        132      142      +10     
Impacted Files Coverage Δ
src/parse_stream.jl 93.89% <92.30%> (-1.53%) ⬇️
src/parser.jl 98.03% <100.00%> (+<0.01%) ⬆️
src/source_files.jl 97.39% <100.00%> (+0.02%) ⬆️

... and 1 file with indirect coverage changes

Diagnostics may sometimes cover trailing text which wasn't consumed. For example,
`parseatom(")")` doesn't consume the errant trailing ')', but the
diagnostic refers to this character. In this case, `SourceFile` needs to
cover the location of the diagnostic in addition to any text which is
consumed.

As part of this, mark `sourcetext(::ParseStream)` as deprecated, in
favour of constructing a `SourceFile` to wrap things up more neatly.

Also fix a bug in `highlight()` for empty `SourceFile`s.
@c42f c42f force-pushed the caf/fix-trailing-diagnostics branch from 6e4e7e2 to fcdd8cf Compare June 25, 2023 23:37
@c42f c42f merged commit 747702b into main Jun 26, 2023
@c42f c42f deleted the caf/fix-trailing-diagnostics branch June 26, 2023 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash formatting errors arising from parseatom
1 participant