odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.13k stars 551 forks source link

Pad ‘^~~~^’-style diagnostic ranges properly #3744

Closed Mango0x45 closed 3 weeks ago

Mango0x45 commented 3 weeks ago

This fixes the immediate issue described in #3743. There are other problems still though, like the fact that this will most definitely display incorrectly if the token the range covers contains non-ASCII, but this is a start!

flysand7 commented 3 weeks ago

I also think that if the character is a tab, the tab should be printed before ^~~^ style diagnostic. Tab width is not a fixed width. Some terminals have it configured at 8 spaces, others at 4.

Mango0x45 commented 3 weeks ago

I also think that if the character is a tab, the tab should be printed before ^~~^ style diagnostic. Tab width is not a fixed width. Some terminals have it configured at 8 spaces, others at 4.

Yeah that’s a good point. I can do that.

Mango0x45 commented 3 weeks ago

The ^~~~^ ranges should now also be printed with the proper width. Previously they would also be messed up if the error token wasn’t ASCII, but this is no longer the case.

Mango0x45 commented 3 weeks ago

I also think that if the character is a tab, the tab should be printed before ^~~^ style diagnostic. Tab width is not a fixed width. Some terminals have it configured at 8 spaces, others at 4.

I thought about it and I think that special handling for tabs is something that wouldn’t make much sense without further discussion, or the addition of some kind of tab-size configuration option. For the leading whitespace before the ^~~~^ range printing tabs makes sense, but the issue comes when printing the actual range itself. How many tildes do I need to print to cover a tab? That depends on the width of your tab.

Kelimion commented 3 weeks ago

MSVC complaints addressed. Tested on Windows and Linux with the växjö example.

Kelimion commented 3 weeks ago

Reverted because of regression.