-
Notifications
You must be signed in to change notification settings - Fork 45
fix off by one error in multiline highlighting #42
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
Conversation
Thank you for the contribution!
It seems like it makes fixture tests fail. @digama0 can you verify that the test positions should be affected by your patch and that the new end indexes are correct?
I see the following follow up PR fixing the tests:
diff --git a/tests/fixtures/no-color/multiline_annotation.toml b/tests/fixtures/no-color/multiline_annotation.toml index bdb577f..c3dc1e9 100644 --- a/tests/fixtures/no-color/multiline_annotation.toml +++ b/tests/fixtures/no-color/multiline_annotation.toml @@ -33,7 +33,7 @@ range = [5, 19] [[slices.annotations]] label = "expected enum `std::option::Option`, found ()" annotation_type = "Error" -range = [22, 765] +range = [22, 766] [title] label = "mismatched types" id = "E0308" diff --git a/tests/fixtures/no-color/multiline_annotation2.toml b/tests/fixtures/no-color/multiline_annotation2.toml index 6ec0b1f..845bf9f 100644 --- a/tests/fixtures/no-color/multiline_annotation2.toml +++ b/tests/fixtures/no-color/multiline_annotation2.toml @@ -10,7 +10,7 @@ fold = false [[slices.annotations]] label = "missing fields `lineno`, `content`" annotation_type = "Error" -range = [31, 127] +range = [31, 128] [title] label = "pattern does not mention fields `lineno`, `content`"
Yes, the new spans are one character too short now but that's because the input span was already compensating for this bug with an end index which ends one character before the actual span. (Was this a real span generated by rustc?) Your suggested fixes put the input spans where they should be, so the output should now be the same as before.
Thank you for adding the tests!
Implements the fix described in #41.
Fixes #41