Re: [RFC 1/1] Add new indent-with-tab whitespace check

Hi. Thanks for the detailed feedback.

Junio C Hamano [off-list ref] writes:

This sounds more like "tab-in-indent".  Naming the option that way would
give it an added benefit of allowing it to be abbreviated to "tab" by the
parser.

Yes that's nice: much harder to confuse with indent-with-non-tab that way.

I think WS_TAB_IN_INDENT are incompatible with either WS_SPACE_BEFORE_TAB
or WS_INDENT_WITH_TAB.  Don't we want to notice such a combination as an
error?

Oh yes, that has to be the right behaviour for the finished feature here. (I
guess strictly they could coexist with the meaning 'no indented lines
allowed at all' but I don't think that's useful or sensible.)

I'll queue this with s/indent-with-tab/tab-in-indent/ tweak to 'pu' but
the topic would need:

 - detecting incompatible settings;
 - tests;
 - docs;
 - matching change to apply --whitespace=fix;

before moving to 'next', I think.

Are you happy with my suggestion in [0/1] to replumb ws_fix_copy() to take a
strbuf as dst rather than a char *, to deal with the fact that whitespace
can grow with --whitespace=fix and WS_TAB_IN_INDENT?

If so, I'll polish up the existing patch as you suggest in your first three
points above, and then make this change to the way ws_fix_copy() works so I
can do the right thing in whitespace=fix.

There is another issue with this change.  Because "whitespace" without any
string in .gitattributes are defined to cause all the whitespace breakages
known to git to be caught, and tab-in-indent is inherently incompatible
with indent-with-non-tab, this cannot be supported without changing the
definition of "default set of whitespace breakage classes".

Ah, this was a behaviour I didn't know about. I should probably put your
patch (without the WS_TAB_IN_INDENT line) as the first one in the series,
before I introduce WS_TAB_IN_INDENT, so we don't break this wildcard
whitespace behaviour at any point in the series.

Cheers,

Chris.