Fix mispositioned error indicators #44386
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
Fixes rust-lang#38384 Most of the Rust community uses 4 spaces for indentation, but there are also tab users of Rust (including myself!). This patch fixes a bug in error printing which mispositions error indicators when near code with tabs. The code attempted to fix the issue by replacing spaces with tabs, but it sadly wasn't enough, as sometimes you may not print spaces but _ or ^ instead. This patch employs the reverse strategy: it replaces each tab with a space, so that the number of _ and ^ and spaces in error indicators below the code snippet line up perfectly. In a study [1] preceeding this patch, we could see that this strategy is also chosen by gcc version 6.3.0. Its not perfect, as the output is not beautiful, but its the easiest to implement. If anyone wants to improve on this, be my guest! This patch is meant as improvement of the status quo, not as perfect end status. It fixes the actual issue of mispositioning error indicators. [1]: rust-lang#38384 (comment)
|
Given that this is not an ideal solution, I feel that we should keep the current output for any situation where there are no multiline spans being displayed, and this output otherwise. This could be problematic when we're displaying two diagnostics, one with a multiline span and one without, as they would look different. This could be alleviated somewhat by modifying |
alexcrichton
added
the
S-waiting-on-review
I'm not sure what you mean with "multiline" spans but the bug already appears in this example. I just want the output to stop being buggy. It might look different from your text editor, so my solution is not perfect, as I'm admitting, but I didn't mean this PR as reaching perfection. Displaying tabs with a red background would make them look like errors themselves, which they are not (its completely fine to use tabs). |
|
I think I would have preferred replacing each tab with four space characters rather than a single space character (and it seems like that was @est31 's original plan here), but I am also a fan of keeping implementations simple. So I'll r+ this now, and independently investigate whether we can further extend this code to do a replacement with four space characters rather than just one. Speaking of simple, this new code is much simpler to understand than the previous algorithm; that's a plus.
|
|
@bors r+ |
|
📌 Commit d582810 has been approved by |
Fix mispositioned error indicators Fixes #38384 Most of the Rust community uses 4 spaces for indentation, but there are also tab users of Rust (including myself!). This patch fixes a bug in error printing which mispositions error indicators when near code with tabs. The code attempted to fix the issue by replacing spaces with tabs, but it sadly wasn't enough, as sometimes you may not print spaces but _ or ^ instead. This patch employs the reverse strategy: it replaces each tab with a space, so that the number of _ and ^ and spaces in error indicators below the code snippet line up perfectly. In a study [1] preceeding this patch, we could see that this strategy is also chosen by gcc version 6.3.0. Its not perfect, as the output is not beautiful, but its the easiest to implement. If anyone wants to improve on this, be my guest! This patch is meant as improvement of the status quo, not as perfect end status. It fixes the actual issue of mispositioning error indicators. [1]: #38384 (comment)
pnkfelix
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
Display `\t` in diagnostics code as four spaces Follow up to #44386 using the unicode variable width machinery from #45711 to replace tabs in the source code when displaying a diagnostic error with four spaces (instead of only one), while properly accounting for this when calculating underlines. Partly addresses #44618.
Fixes #38384
Most of the Rust community uses 4 spaces for indentation,
but there are also tab users of Rust (including myself!).
This patch fixes a bug in error printing which mispositions
error indicators when near code with tabs.
The code attempted to fix the issue by replacing spaces
with tabs, but it sadly wasn't enough, as sometimes
you may not print spaces but _ or ^ instead.
This patch employs the reverse strategy: it replaces each
tab with a space, so that the number of _ and ^ and spaces
in error indicators below the code snippet line up
perfectly.
In a study 1 preceeding this patch, we could see that
this strategy is also chosen by gcc version 6.3.0.
Its not perfect, as the output is not beautiful, but its
the easiest to implement. If anyone wants to improve on
this, be my guest! This patch is meant as improvement of
the status quo, not as perfect end status. It fixes the
actual issue of mispositioning error indicators.