You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SourceAnnotation underline displays offset from range #29
Oh, #27 didn't fix the bug properly. As I said here, one cannot use .lines() for this.
The relevant change in #27 is that they removed the "+1" in this line:
What this does is that instead of treating each line ending as 2 chars, they treat it as 1 char. But the correct fix is to distinguish between the different types of line endings.
Generally I don't think char indexing is really what you want anyways, because unicode makes everything complicated.
This is the exact same issue as in #24, except that in #24 it failed on \n line endings and succeeded on \r\n endings, but now it's the reverse. I don't have code that triggers this available, but any code with a line ending in \r\n and a snippet pointing somewhere after that line should show the error.
The other cases without "\r\n" as the end of the line, have you seen any other problem?
Coverage is limited and I don't know how it can work in certain extreme cases.
I would greatly appreciate your help detecting these extreme cases and adding coverage since I want to use it in my template engine, thanks !!
The patch looks good ! As far as I'm concerned this is fixed.
I didn't encounter that particular problem in real code however, I was just helping. @gipsond can you try master ? It should be fixed.
Activity
Nadrieril commentedon Mar 31, 2020
Oh, #27 didn't fix the bug properly. As I said here, one cannot use
.lines()for this.The relevant change in #27 is that they removed the "+1" in this line:
annotate-snippets-rs/src/display_list/from_snippet.rs
Line 186 in 7e73a1b
What this does is that instead of treating each line ending as 2 chars, they treat it as 1 char. But the correct fix is to distinguish between the different types of line endings.
Generally I don't think char indexing is really what you want anyways, because unicode makes everything complicated.
zzau13 commentedon Apr 3, 2020
@Nadrieril Can you post the
Snippetand expected output?Nadrieril commentedon Apr 3, 2020
This is the exact same issue as in #24, except that in #24 it failed on
\nline endings and succeeded on\r\nendings, but now it's the reverse. I don't have code that triggers this available, but any code with a line ending in\r\nand a snippet pointing somewhere after that line should show the error.Nadrieril commentedon Apr 3, 2020
Actually, here you go:
Output:
Expected:
The shift gets linearly worse when there are more lines.
zzau13 commentedon Apr 3, 2020
Ok it is easy to solve.
The other cases without
"\r\n"as the end of the line, have you seen any other problem?Coverage is limited and I don't know how it can work in certain extreme cases.
I would greatly appreciate your help detecting these extreme cases and adding coverage since I want to use it in my template engine, thanks !!
Nadrieril commentedon Apr 3, 2020
This is the only problem I am aware of
Merge pull request #32 from botika/master
zbraniecki commentedon Apr 4, 2020
@Nadrieril can you try master! @botika wrote a patch for the line endings!
Nadrieril commentedon Apr 4, 2020
The patch looks good ! As far as I'm concerned this is fixed.
I didn't encounter that particular problem in real code however, I was just helping. @gipsond can you try master ? It should be fixed.
zbraniecki commentedon Apr 16, 2020
I released
0.8. Please, comment if the bug still exists.chore(deps): Update Rust Stable to v1.89 (rust-lang#29)