Account for tabs when highlighting multiline code suggestions

[estebank]

Address '\t' case in #87972.

Before:

After:

[rust-highfive]

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

[m-ou-se]

I think this should just be 1. Counted in chars and not in columns.

Current nightly:

This PR:

(The second 'A' is a double-width 'Ａ'.)

[estebank]

Can you verify the current version of the PR against your test cases? Sadly, we don't currently have regression tests for the coloring of the output in our tests :-/

This is what I'm seeing in my local tests:

[m-ou-se]

Yup, the highlighting looks good now!

[estebank]

In particular, I want to confirm that the output of the terminal without emoji support you showed here is reasonable (as I fully expect it to be, because it relies on what we were doing for the multiline case, which looks ok in that screenshot).

[estebank]

Sadly, now I'm seeing other issues :(

[m-ou-se]

Here's the output in four more terminals:

[cjgillot]

Can you take care of this PR @m-ou-se?

[estebank]

@m-ou-se do let me know if there's anything left here. I want to avoid this not reaching master before the next beta cutoff.

[m-ou-se]

I found one more (unlikely) edge case (which is not very important, but not hard to fix), and a have a small comment on the code.

r=me either way

[m-ou-se]

This assumes the addition/replacement doesn't contain tabs itself. It rarely happens, but it can occur:

Here, the entire closure body up to and including the final } is supposed to be highlighted, but i put some tabs in the string literal which broke it.

[estebank]

Ah, yes. I assumed that we didn't have tabs in the suggestion because I didn't account for cases where snippets from the user include them :-/

[m-ou-se]

I suppose all cases where user snippets are included should be replaced by multispan suggestions. (This specific one is already fixed by #87958, but I think there's quite a few places left where this happens.)

[estebank]

Could you give me the repro case? I think I fixed this, but the suggestions seem to be doing the right thing for me.

[m-ou-se]

With #87958 merged, the test case in the screenshot above doesn't trigger the problem anymore. We'll have to find another case where a bunch of user code gets put verbatim in the suggestion. Grepping for span_to_snippet now. ^^

[m-ou-se]

Found one:

mod a {pubconstA:String = String::new();}fnmain(){
a
::A;//^ TAB here}

[estebank]

The current code handles this correctly:

[estebank]

@bors r=m-ou-se

[bors]

📌 Commit 955e913 has been approved by m-ou-se

[camsteffen]

I'm getting this when I run ./x.py test src/tools/clippy (line 345 in this PR diff)

thread 'rustc' panicked at 'attempt to subtract with overflow', compiler/rustc_errors/src/lib.rs:347:47

[estebank]

@camsteffen it sounds like there might be a malformed Span involved, where the end is earlier than the end orlen is smaller than the Span... Although now that I see it the if might be incorrect (we compare prev_hi and cur_lo but then calculate using cur_hi and cur_lo. What are the steps to reproduce this, just running that command?

[camsteffen]

Yes I am seeing that error on multiple Clippy UI tests just running that command on 4878034 (and probably current master).

[estebank]

I wasn't able to reproduce this :-/

[camsteffen]

Well shoot. Must've been an artifact of my local setup. I'm not seeing it either and it hasn't shown up in CI.