Skip to content
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

Support . as a row:column separator in terminal link detector #190351

Merged
merged 2 commits into from Aug 14, 2023

Conversation

Timmmm
Copy link
Contributor

@Timmmm Timmmm commented Aug 13, 2023

The motivation here is the Sail language compiler which outputs errors conforming to the GNU style

https://www.gnu.org/prep/standards/html_node/Errors.html

I think they must be the only people in the world actually using the line0.col0-line1.col1 format. I did have an attempt to capture the ending line/column but it is very difficult with regex, and not that useful anyway so I've opted for the simpler option of just ignoring the - part.

Fixes #190350

The easiest way to test this is to make a file test.txt containing something like this:

Error: some error test.txt:1.10

Then cat it in the integrated terminal and click the links.

The motivation here is the Sail language compiler which outputs errors conforming to the GNU style

https://www.gnu.org/prep/standards/html_node/Errors.html

I think they must be the only people in the world actually using the `line0.col0-line1.col1` format. I did have an attempt to capture the ending line/column but it is very difficult with regex, and not that useful anyway so I've opted for the simpler option of just ignoring the `-` part.
Tyriar
Tyriar previously approved these changes Aug 13, 2023
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great PR! LGTM provided the tests pass

I think they must be the only people in the world actually using the line0.col0-line1.col1 format. I did have an attempt to capture the ending line/column but it is very difficult with regex, and not that useful anyway so I've opted for the simpler option of just ignoring the - part.

We don't do have one that interleaves like that yet, but you might be able to add a new lineAndColumnRegexClauses that does something like this:

"${r()}\.${c()}-${re}\.${ce}"

@Timmmm
Copy link
Contributor Author

Timmmm commented Aug 13, 2023

Wow thanks for the fast review!

I did think about adding that one but then I saw that it would involve adding a new regex and changing the code that reads the capture groups and I wasn't even sure VSCode actually uses the end lines/columns... Maybe that's just an excuse for my laziness :-)

@Tyriar
Copy link
Member

Tyriar commented Aug 13, 2023

A new regex is fine for the class of links that contain them interleaved like that, I expect it to work fine with the new regex. Feel free to leave this PR as is and create a new one to include that (or not bother, no commitment).

@Tyriar Tyriar merged commit 35e0b75 into microsoft:main Aug 14, 2023
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support GNU style file:line.column links
3 participants