[ty] Fix rendering of long lines in diagnostic messages that are indented with tabs #18962
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This converts the MRE in #670 into a fixture test for `annotate-snippets`.
It turns out that `annotate-snippets` doesn't do a great job of consistently handling tabs. The intent of the implementation is clearly to expand tabs into 4 ASCII whitespace characters. But there are a few places where the column computation wasn't taking this expansion into account. In particular, the `unicode-width` crate returns `None` for a `\t` input, and `annotate-snippets` would in turn treat this as either zero columns or one column. Both are wrong. In patching this, it caused one of the existing `annotate-snippets` tests to fail. I spent a fair bit of time on it trying to fix it before coming to the conclusion that the test itself was wrong. In particular, the annotation ranges are 4 bytes off. However, when the range was wrong, the buggy code was rendering the example as intended since `\t` characters were treated as taking up zero columns of space. Now that they are correctly computed as taking up 4 columns of space, the offsets of the test needed to be adjusted. Fixes #670
BurntSushi
added
bug
That's a somewhat "recent" change in unicode-width, I think it used to return 1. So maybe that used to work |
MichaReiser
approved these changes
Jun 26, 2025
|
Uff, that sounds awful to debug. Thanks for tracking this down |
|
I think this was for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It turns out that
annotate-snippetsdoesn't do a great job ofconsistently handling tabs. The intent of the implementation is clearly
to expand tabs into 4 ASCII whitespace characters. But there are a few
places where the column computation wasn't taking this expansion into
account. In particular, the
unicode-widthcrate returnsNonefor a\tinput, andannotate-snippetswould in turn treat this as eitherzero columns or one column. Both are wrong.
In patching this, it caused one of the existing
annotate-snippetstests to fail. I spent a fair bit of time on it trying to fix it before
coming to the conclusion that the test itself was wrong. In particular,
the annotation ranges are 4 bytes off. However, when the range was
wrong, the buggy code was rendering the example as intended since
\tcharacters were treated as taking up zero columns of space. Nowthat they are correctly computed as taking up 4 columns of space, the
offsets of the test needed to be adjusted.
Fixes astral-sh/ty#670