Compute widths properly when displaying spans in error messages #21499
Conversation
| let mut s = String::new(); | ||
| // Skip is the number of characters we need to skip because they are | ||
| // part of the 'filename:line ' part of the previous line. | ||
| let skip = fm.name.len() + digits + 3u; | ||
| let skip = fm.name.width(false) + digits + 3u; |
There was a problem hiding this comment.
From the documentation: "Unicode Standard Annex #11 recommends that these characters be treated as 1 column (i.e., is_cjk = false) if the context cannot be reliably determined."
(It actually greatly varies on the font settings, so it cannot be reliably set by default.)
ftxqxd
force-pushed
the
issue-8706
branch
2 times, most recently
from
c187fcc to
8c7b4ad
Compare
| try!(write!(&mut err.dst, "{}", s)); | ||
| let mut s = String::from_str("^"); | ||
| let count = match lastc { | ||
| // Most terminals render tabs as 8 spaces |
There was a problem hiding this comment.
As for the terminals (as opposed to editors), this might be true, though I don't have no backing data.
There was a problem hiding this comment.
Tab control characters are not rendered. They move the cursor to the next tab stop.
There was a problem hiding this comment.
Ah, indeed. Understood what you did mean.
There was a problem hiding this comment.
I’ve adjusted this bit to assume that all tab stops are 8 spaces apart and render the span accordingly, rather than just unconditionally inserting 8 spaces.
There was a problem hiding this comment.
Could you update this comment to refer to tabstops explicitly?
|
r? @huonw |
|
It'd be nice if there was a regression test for this |
|
I agree that it would be nice for a regression test. I think the most reliable way to test this would be a r=me with the comment tweak and a test. |
|
Needs a rebase, but that test looks good, so r=me. |
|
@huonw Rebased. |
|
@bors r+ d244 |
Closes #8706.