Render \t as 8 spaces in caret diagnostics
ClosedPublic

Authored by Rufflewind on May 8 2017, 4:12 PM.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Rufflewind created this revision.May 8 2017, 4:12 PM
Rufflewind retitled this revision from Summary: Render \t as 8 spaces in caret diagnostics to Render \t as 8 spaces in caret diagnostics.May 8 2017, 4:14 PM
Rufflewind edited the summary of this revision. (Show Details)
bgamari edited edge metadata.May 8 2017, 4:33 PM

Perhaps we should have a test for this?

rwbarton requested changes to this revision.May 8 2017, 5:00 PM
rwbarton added inline comments.
compiler/main/ErrUtils.hs
281–287

Please add some comments about why this is necessary (with a reference to the ticket number).

283

Technically a tab character should advance to the next multiple of 8 relative to the start of the original source line. Since non-leading tabs are rather rare, it seems fine to not implement this but it would be good to include a comment about the fact that you don't.

This revision now requires changes to proceed.May 8 2017, 5:00 PM
Rufflewind planned changes to this revision.May 8 2017, 5:08 PM

Hm, it seems testing is not as straightforward – I think the stderr normaliser turns tabs into 8 spaces? So it can't really tell the difference between the two…

Rufflewind updated this revision to Diff 12438.May 8 2017, 6:12 PM
Rufflewind edited edge metadata.

Fix fixWhitespace and also the tests

Rufflewind marked 2 inline comments as done.May 8 2017, 6:16 PM

I tweaked the testlib a little bit to allow changing the whitespace-normaliser.

Also, how do you silence the linter?

compiler/main/ErrUtils.hs
283

This was an oversight and has been fixed.

bgamari accepted this revision.May 9 2017, 12:25 PM

Looks good to me.

This revision is now accepted and ready to land.May 9 2017, 12:25 PM

Also, how do you silence the linter?

It's unfortunately not easy. Just ignore him.

This revision was automatically updated to reflect the committed changes.
rwbarton added inline comments.May 12 2017, 5:45 PM
compiler/main/ErrUtils.hs
284

Er, wait. Won't this turn a line that starts with "\t\t" into 8+7 spaces? That seems even worse than before.

Rufflewind marked an inline comment as done.May 12 2017, 7:22 PM
Rufflewind added inline comments.
compiler/main/ErrUtils.hs
284