Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Fix origin position computation #154

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
Muscraft merged 2 commits into rust-lang:master from KarelPeeters:fix-origin
Oct 22, 2024
Merged

Conversation

Copy link
Contributor

@KarelPeeters KarelPeeters commented Oct 13, 2024

This PR fixes an issue in the computation of the position in the origin file. When a highlighted section starts at the beginning of the line, the origin position used to point to the end of the previous line, instead of the start of the current line. This resulted in outputs like this:

error: title
 --> origin.txt:2:4
 |
1 | aaa
2 | bbb
3 | ccc
 | ^^^ annotation
4 | ddd
 |

Here origin.txt:2:4 is wrong, it should be origin.txt:3:1, which actually matches the highlighted setting.

As a drive-by fix, 21645ad fixes EndLine, which was used incorrectly in multiple places that happened to cancel out.

ZedThree reacted with hooray emoji
Copy link
Contributor Author

Ah, I missed those failing test cases, since they're disabled on windows:

fn main() {
#[cfg(not(windows))]
tryfn::Harness::new("tests/fixtures/", setup, test)
.select(["*/*.toml"])
.test();
}

Is there any reason for that, maybe CRLF issues? They seem to behave as expected for me when I just remove that #[cfg(not(windows))] line!

The failing test are about cases that try to point one byte past the end of the line. Previously that was indeed possible, but only in very limited situations, eg. the highlighting only worked on EOF cases anyway. See #108 (comment) for vaguely relevant discussion about this.

I'm not sure what the best way to proceed here is. Is the "one past the end of the line" use case already adequately supported by just allowing users to point at the newline characters themselves? Ideally there would be some out-of-band way to signal this case, since only using byte indices for this is tricky.

Copy link
Member

Is there any reason for that, maybe CRLF issues? They seem to behave as expected for me when I just remove that #[cfg(not(windows))] line!

I believe we disabled the test harness on Windows because there were issues with the test harness running on windows.

@KarelPeeters KarelPeeters force-pushed the fix-origin branch 2 times, most recently from 87c8d40 to 9e673e0 Compare October 16, 2024 21:09
Copy link
Contributor Author

I think I've addressed all of the comments, and I have rebased this PR on the current master. Let me know if I can do anything else to improve this PR!

@Muscraft Muscraft merged commit e41310d into rust-lang:master Oct 22, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Muscraft Muscraft Muscraft approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /