Count chars when creating SourceAnnotation#6084
Closed
MarcusGrass wants to merge 4 commits into
Closed
Conversation
Contributor
|
@MarcusGrass thanks for looking into this! I came to the same conclusion regarding chars vs bytes when I looked into this a while ago. I'll follow up when I've had a chance to take a look at this / refresh my memory on what I was looking into previously. This has been a long standing issue and I'd be very glad if we could fix it! |
Contributor
|
Closing this as #6391 was merged, and the newer version of annotated_snippet uses byte offsets instead of char indexes. See #6391 (comment) |
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.
Fixes #6083.
In annotate snippets it takes the length of the string by char count https://github.com/rust-lang/annotate-snippets-rs/blob/master/src/renderer/display_list.rs#L935, both on
0.9and0.10, but in the code here thelength is taken by byte-index.
Since the user's string has chars that do not correspond 1-to-1 with byte-indices there's a range-mismatch and a following panic.
It only occurs in some cases and was fairly hard to reproduce, I think it has something to do with max line-count as well, and that's when it blows up.
I'm not so familiar with this repo,
I can't get the test to pass. The source can't be formatted, since the comment would be lost, but if that's disabledrustfmtwill err on trailing whitespace and I don't know if that can be disabled, but the test really checks thatrustfmtdoesn't panic, so maybe it just doesn't fit into this mould.E: I put the test-file under its own directory and added it to the dont_emit_ice_test instead.
Can easily be reproduced by running
rustfmt tests/issue-6083/issue_6083.rsvscargo run --bin rustfmt -- tests/issue-6083/issue_6083.rson this branch.