Re: [PATCHv3] git apply: option to ignore whitespace differences

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 31, 2009, 17:17 UTC
Message-ID: <7v1vnwsreu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <cb7bb73a0907310916i309c99cax2f2d5aa02084103f@mail.gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

Show 12 quoted lines

> Before I go on with the next revision of the patch, I would like to
> have some kind of agreed convention to implement.
>
> My suggestion would be the following:
> (1) implement options --ignore-space-change, --ignore-all-space
> mirroring their 'git diff' meaning.
> (2) add --ignore-whitespace as a synonym to --ignore-space-change, for
> consistency with 'patch'
> (3) apply.ignore-whitespace accepts values
>      * false,no,none,0 to mean no whitespace ignoring
>      * true,yes,change,1 to mean ignore whitespace change
>      * all,2 to mean ignore all whitespace

I'd suggest taking one small bite at a time. You do not have code to do the ignore-all-space semantics that has been reviewed, and you neither have the configuration variables. So my preference would be to do in the first round the first half of (1) with (2), docs, tests and nothing else.

A later follow-up patch would complete your (1) and (2) by supporting ignore-all-space. And perhaps you would give it --ignore-all-whitespace synonym perhaps? You may want to ask "GNU patch" people if they are interested in ignoring all whitespaces, and if so what their plan is to name that option, so that you can use the same name.

As to configuration:

 - I think the naming convention (I know, there are existing violators,
   but that is not an excuse to add new ones) is without dashes (again, I
   know I personally do not like CamelCase but that is what we have);

 - I personally think 0/1/2 would be more cluttering than they are useful.

 - I'd say "no (never, none)", "change", "all".  You could throw in
   "false" if you want, as declining this option is quite boolean-ish, but
   activating it is _not_ boolean decision and I would suggest actively
   keeping "true" or "yes" out of the choices.  Otherwise anybody who is
   tempted to answer "yes" has to ask himself "Ok, I want to say 'yes' but
   which semantics does it mean?  What would I be ignoring?"

That non-booleanness of "true" is why I am somewhat negative on your (2) above. It would only be there for people who know "GNU patch" because they would understand it.