From: Junio C Hamano <gitster@pobox.com>
To: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
Nanako Shiraishi <nanako3@lavabit.com>,
git@vger.kernel.org
Subject: Re: [PATCHv3] git apply: option to ignore whitespace differences
Date: Fri, 31 Jul 2009 10:17:45 -0700 [thread overview]
Message-ID: <7v1vnwsreu.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <cb7bb73a0907310916i309c99cax2f2d5aa02084103f@mail.gmail.com> (Giuseppe Bilotta's message of "Fri\, 31 Jul 2009 18\:16\:43 +0200")
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> 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.
next prev parent reply other threads:[~2009-07-31 17:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-28 21:00 [PATCHv3] git apply: option to ignore whitespace differences Giuseppe Bilotta
2009-07-28 21:29 ` Junio C Hamano
2009-07-29 6:33 ` Giuseppe Bilotta
2009-07-29 7:09 ` Junio C Hamano
2009-07-29 8:20 ` Giuseppe Bilotta
2009-07-29 8:39 ` Junio C Hamano
2009-07-29 9:05 ` Giuseppe Bilotta
2009-07-29 8:40 ` Nanako Shiraishi
2009-07-29 9:09 ` Giuseppe Bilotta
2009-07-31 0:27 ` Felipe Contreras
2009-07-31 0:48 ` Junio C Hamano
2009-07-31 15:38 ` Felipe Contreras
2009-07-31 16:16 ` Giuseppe Bilotta
2009-07-31 17:17 ` Junio C Hamano [this message]
2009-07-31 19:22 ` Giuseppe Bilotta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7v1vnwsreu.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=giuseppe.bilotta@gmail.com \
--cc=nanako3@lavabit.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).