Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options.

[Junio C Hamano <gitster@pobox.com>]

David Symonds <dsymonds@gmail.com> writes:

> Each of the new core.whitespace.* options (enumerated below) can be set to one
> of:
> 	* okay (default): Whitespace of this type is okay
> 	* warn: Whitespace of this type should be warned about
> 	* error: Whitespace of this type should raise an error
> 	* autofix: Whitespace of this type should be automatically fixed

Many problems at the conceptual level (I haven't look at the
patch yet).

We call these options (nowarn,warn,error,strip) in
apply.whitespace.  "strip" is a bit of misnomer, as we only
handled the trailing whitespace initially.  We should add "fix"
as a synonym to "strip".

The intention is to define what is an anomaly with
core.whitespace and then define what to do with it with
apply.whitespace.

And that is a good distinction.  You may usually use "fix", but
occasionally you would want to override it one-shot (when you do
want to have byte-to-byte identical application of the patch),
and the command line option "--whitespace=" lets you do so.
At least you need to extend --whitespace command line option
handling to allow these overridden.

Adding the "error" and "fix" to "diff" is a mistake --- there is
no error condition nor fixing there.  That shows how the
approach of your patch is inappropriate by trying to mix what
core.whitespace (give the definition of what is an error) and
apply.whitespace (specify what to do with an error) are designed
to do.

Defaulting to "nowarn" is wrong.  Trailing whitespace errors and
space before tab errors should be turned on by default as
before.