- Notifications
You must be signed in to change notification settings - Fork48
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.tomlindex 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.tomlindex 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