George Papanikolaou <g3orge.app@xxxxxxxxx> writes:
> On Tue, Mar 25, 2014 at 6:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> As a tangent, I have a suspicion that the current implementation may
>> be wrong at the beginning of the string. Wouldn't it match " abc"
>> and "abc", even though these two strings shouldn't match?
>
> Wouldn't that be accomplished by just removing the leading whitespace check?
Yes. I was wondering *what* semantics we want in the first place;
how to implement what I suggested is so trivial that it goes without
saying for the intended audiences of that comment ;-).
> I'm somewhat confused about what the function should match. I haven't
> grasped it.
This function is used when attempting to resurrect a patch that is
whitespace-damaged. The patch may want to change a line "a_bc" in
the original into something else [*1*], and we may not find "a_bc"
in the current source, but there may be "a__bc" (two spaces instead
of one the whitespace-damaged patch claims to expect). By ignoring
the amount of whitespaces, it forces "git apply" to consider that
"a_bc" in the broken patch meant to refer to "a__bc" in reality.
I _think_ the original motivation of ignore_ws_change was to match
the "--ignore-space-change" option of "diff", i.e. "ignore changes
in the amount of white space". I just checked the source
(xdiff/xutils.c) and made sure that "git diff" does not treat the
beginning of line any differently hence "_a_bc" and "a_bc" are not
considered a match under its --ignore-space-change option.
The current implementation of "apply --ignore-space-change" that
ignores leading whitespaces (not "ignore changes in the amount of
leading whitespaces") is likely to be a bug from this point of view.
But I wanted to hear opinions from other Git experts [*2*]. Hence
my "As a tangent, I have a suspicion".
[Footnote]
*1* This mode is not enabled by default. I am not even sure if
anybody sane would (or should) use this option. Sure, the fuzzy
match may be able to find the original line that the patch
author may meant to patch even when it is whiltespace-damaged
because it does not fully trust what the original lines exactly
say (i.e. context lines prefixed by " " and old lines prefixed
by "-"). What makes it sane for us to trust what the
replacement lines (i.e. new lines prefixed by "+") in such a
mangled patch says?
*2* For example, somebody may be able to point out that "this is
meant to match the option of the same name 'diff' has", which is
my assumption that leads to the above discussion, may not be
true.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html