Uh oh!
There was an error while loading. Please reload this page.
Display \t in diagnostics code as four spaces - #45953
Conversation
estebank
commented
Nov 13, 2017
est31
commented
Nov 13, 2017
@estebank nice PR! It definitely improves the behaviour of rustc regarding tabs :). However, tab == 4 spaces replacements (what this PR is doing) are not a emulation of what clang is doing. Instead, clang emulates a tabstop algorithm, meaning that a tab character jumps to the next tabstop. Tabstops are being placed at all 4 spaces. The basic algorithm is: add 1 space, then add another The difference is not observable if you only use tabs at the start of a line and once you have a non tab character you never use them again (which is how I structure all of my own code). You can only see the difference when there is a mix of tabs and spaces. With the tabs == 4 spaces algorithm, With the tabstops algorithm that clang is using, the sequences would become: As I've said above, it wouldn't make any difference for my own code, but it would maybe make a difference for other users. |
estebank
commented
Nov 13, 2017
@est31 I understand. I've changed the description from fixing the bug to merely CCing it. Supporting the proper tab stop algorithm will require some extra code in order for underlines to be correct as well. I feel that for now this is a good stop gap improvement on your code, even if not entirely correct as you point out. |
est31
commented
Nov 13, 2017
Definitely! Really like this PR :) |
shepmaster
commented
Nov 18, 2017
Ping from triage, @pnkfelix — will you have time to look at this soon? |
bors
commented
Nov 24, 2017
☔ The latest upstream changes (presumably #46116) made this pull request unmergeable. Please resolve the merge conflicts. |
kennytm
commented
Nov 29, 2017
Triage ping — @pnkfelix could you take a look at this PR? Thanks! cc @rust-lang/compiler |
estebank
commented
Nov 29, 2017
r? @arielb1 |
kennytm
commented
Dec 6, 2017
Triage ping — @arielb1 could you take a look at this PR? Thanks! |
arielb1
commented
Dec 6, 2017
I don't know this code. r? @nikomatsakis |
| tab_pos.push(pos); | ||
| } | ||
| } | ||
| // start with the tabs at the end of the line to replace them with 4 space chars |
There was a problem hiding this comment.
Am I missing something...? Why is it useful/important to go in reverse? (Doesn't seem harmful, but also not imp't.)
There was a problem hiding this comment.
oh, I see, duh. Otherwise the indices are invalid.
nikomatsakis
left a comment
There was a problem hiding this comment.
Looks good to me. r=me.
nikomatsakis
commented
Dec 6, 2017
@bors r+ |
bors
commented
Dec 6, 2017
📌 Commit 9d80e22 has been approved by |
bors
commented
Dec 6, 2017
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.
bors
commented
Dec 6, 2017
☀️ Test successful - status-appveyor, status-travis |
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.