Re: [PATCHv3] git apply: option to ignore whitespace differences
- From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
- Date
- Jul 29, 2009, 06:33 UTC
- Message-ID
- <cb7bb73a0907282333g26efd1d8y7d913fba8a426aa5@mail.gmail.com>
- In-Reply-To
- <7vljm84htf.fsf@alter.siamese.dyndns.org>
On Tue, Jul 28, 2009 at 11:29 PM, Junio C Hamano<gitster@pobox.com> wrote:
Show 13 quoted lines
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes: > >> @@ -205,6 +209,9 @@ running `git apply --directory=modules/git-gui`. >> Configuration >> ------------- >> >> +apply.ignore-whitespace:: >> + Set this boolean configuration flag if you want to ignore whitespace >> + differences to be ignored by default. > > That's a strange sentence. > > When set to true, changes in amount of whitespace are ignored.
Indeed, my sentene was horrible.
Show 6 quoted lines
> I am not happy with the name --ignore-whitespace. > > Perhaps --ignore-space-change, to be consistent with a "git diff" option, > would be more appropriate. Doing so has an added benefit of leaving the > door open to add --ignore-all-space option to the patch application side > later.
On the other hand, --ignore-whitespace matches the option name (and behavior) of the 'patch' command (just like "git diff"'s matches the 'diff' option name and behavior). Principle of least surprise says that someone coming to git from raw diff/patch setups would expect --ignore-whitespace on the patch side.
A possible future expansion to allow ignoring all whitespace completely would be to allow --ignore-whitespace=all
> If I am reading the patch correctly, use of this option always disables > checks and corrections of whitespace errors.
Not exactly. Whitespace correction is not attempted to try matching context lines, but the whitespace correction on the new lines is still applied.
> I personally feel that > somebody who is willing to accept a patch that has whitespace breakages > that needs this option would not care
Indeed, my first patch fell through to the whitespace correction, I changed it per your suggestion.
> but the documentation should warn > about it at the minimum.
The code comments does mention that context line whitespace correction that will be skipped. I'll add a note about it in the man page.
Show 5 quoted lines
> Needless to say, a lot better option is not to disable the checks and > corrections at all. After all, this "ignore space change" option is about > matching the common context lines and replaced/removed contents before the > change, and checks and corrections are about added/replaced contents after > the change. They should be orthogonal.
They are, for new lines.
Actually, one thing that I've been thinking about doing is to adjust the new lines to match the kind of whitespace change the context line underwent. Example: if all the context lines had the change 4 spaces -> tab, it would be nice to have the new lines undergo the same change. However, this is going to be rather hard to implement.
-- Giuseppe "Oblomov" Bilotta