Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#9723 closed feature request (fixed)

Give Tab warning only once per file

Reported by: nomeata Owned by: dalaing
Priority: low Milestone: 8.0.1
Component: Compiler Version: 7.9
Keywords: newcomer Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: parser/should_compile/T9723{a,b}
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D760
Wiki Page:

Description

Currently, when compiling a tabbed file, GHC spits out a huge number of warnings:

Main.hs:1:1: Warning: Tab character

[..]

Main.hs:89:1: Warning: Tab character

Main.hs:90:1: Warning: Tab character

Main.hs:91:1: Warning: Tab character

Main.hs:92:1: Warning: Tab character

Main.hs:99:4: Warning: Tab character

Main.hs:100:4: Warning: Tab character

Main.hs:101:4: Warning: Tab character

This is annoying and not very nice to our users.

I suggest to replace it by

This file contains tabs (e.g. line n column m); please use spaces instead

or (to be nice to tools that parse this output)

Main.hs:1:1: Tab found here, and in n further locations. Please use spaces instead.

(This is clearly a low priority request, but also possibly some low-hanging fruit for new contributors.)

Change History (11)

comment:1 Changed 3 years ago by thomie

Keywords: newcomer added

comment:2 Changed 3 years ago by dalaing

Owner: set to dalaing

comment:3 Changed 3 years ago by dalaing

Differential Rev(s): Phab:D760
Status: newpatch

comment:4 Changed 3 years ago by hvr

I'm a bit dubious about this change. While I understand the intent it seems to the first warning which is not emitted at all occurrences. So tooling can't highlight all offending lines at once, and would need to be made aware that this warning actually affects more lines.

So maybe the compromise would be to have a flag to enable warning *all* tab-polluted lines, but have it default to warn only about the first occurence

comment:5 Changed 3 years ago by goldfire

While I generally agree with @hvr in wanting to think about tooling, I disagree in this instance. It's very easy for tooling to find tabs on its own -- much easier, I would bargain, than trying to parse GHC's warnings. And maintaining two different warnings, with their own flags, for "warn once" vs. "warn every time" seems heavy.

comment:6 Changed 3 years ago by nomeata

So tooling can't highlight all offending lines at once, and would need to be made aware that this warning actually affects more lines.

This is a good thing. If you edit a file with tabs in every line, highlighting every line is more annoying than helpful, IMHO.

comment:7 Changed 3 years ago by dalaing

I was a bit torn as well. I mostly liked the idea of picking some low hanging fruit to get into GHC hacking :)

I like using the go-to-previous-error and go-to-next-error features in my editor, and if a few tabs have crept in it'd be nice to just fix them up in my usual cycle.

On the other hand, most of the tabs I've come across in the checkout of GHC have been in files where tabs are used consistently throughout the file (time, xhtml, ...) - in which case I'd prefer one warning after which I'd get my editor to retab the file and move on.

comment:8 Changed 3 years ago by Thomas Miedema <thomasmiedema@…>

In afcfb62b748c41d31b8c8e3ef7f623fa00a1cfd2/ghc:

Change 'Tab character' warnings so there is one per file (#9723)

Reviewed By: nomeata, thomie

Differential Revision: https://phabricator.haskell.org/D760

Signed-off-by: Dave Laing <dave.laing.80@gmail.com>

comment:9 Changed 3 years ago by thomie

Milestone: 7.12.1
Resolution: fixed
Status: patchclosed
Test Case: parser/should_compile/T9723{a,b}

Yeah!

comment:10 Changed 2 years ago by Thomas Miedema <thomasmiedema@…>

In ced27def2d33119ed9fcc22f92856f132fd72217/ghc:

Remove dead code / overlapping pattern (#9723)

comment:11 Changed 2 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

Note: See TracTickets for help on using tickets.