Skip to content

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

Merged
merged 2 commits into from
Mar 2, 2021

Conversation

digama0
Copy link
Contributor

@digama0 digama0 commented Mar 1, 2021

Implements the fix described in #41.

Fixes #41

@zbraniecki
Copy link
Contributor

Thank you for the contribution!

@zbraniecki
Copy link
Contributor

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?

@zbraniecki
Copy link
Contributor

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`"

@digama0
Copy link
Contributor Author

digama0 commented Mar 2, 2021

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.

@zbraniecki zbraniecki merged commit a0e10a8 into rust-lang:master Mar 2, 2021
@zbraniecki
Copy link
Contributor

Thank you for adding the tests!

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.

Off-by-one error in multiline error highlights
2 participants