Fix formatting bug when an empty span is not aligned to a char boundary #314
Conversation
Previous output looked like this: ---- single_line_with_wide_char_unaligned_span_empty stdout ---- Error: oops::my::bad × oops! ╭─[bad_file.rs:2:4] 1 │ source 2 │ 👼🏼text · ─▲ · ╰── this bit here 3 │ here ╰──── help: try doing it better next time? Note that the .max(start + 1) term is still necessary in the nonempty branch, since it's possible to have a nonempty span covering zero-width text.
start > end in all cases.
| } | ||
| underlines.push_str( | ||
| &format!( | ||
| "{:width$}{}{}{}", |
There was a problem hiding this comment.
I just fixed a bug in #316 that also uses these :width$ format specifiers. It seems that they're not such a good idea when working with ansi. I do see that you apply style after you do that here though, so it might work fine here actually. I just noticed it in this diff as well and maybe this should be checked everywhere 🤔
There was a problem hiding this comment.
I believe it's fine in this case, since the actual format arg is "". format!("{:width$}", "", width = x) should just expand to x space chars. I might be misunderstanding what the problem is, but I don't think this is the same bug as the one you found in #316.
There was a problem hiding this comment.
ohh yes that is correct, I do that too somewhere I think. You're right, this works :D
This bug was originally identified in #312 (comment).
The bad output looked like this:
Another possible option for the output (other than the one I implemented) would be
I prefer pointing the
▲at the first column of the wide char rather than the second, but can see an argument that pointing at the second column is visually communicates that the span is unaligned. I think that's pretty niche, and would be surprised if any users are generating unaligned spans on purpose.I also included a minor refactor in the PR, removing an unnecessary branch. The branch was always taken both before and after the bugfix. I can remove that bit if you want.