Re: [PATCH v2 2/4] apply: Allow blank context lines to match beyond EOF

["Björn Gustavsson" <bgustavsson@gmail.com>]

2010/2/24 Junio C Hamano <gitster@pobox.com>:
> Very nicely done.

Thanks! :)

> On the other hand "limit" does not have such a good definition, other than
> as a work around to bypass line-number check when we are trying to match
> at the end.  It might be cleaner to read if we move the problematic "line
> numbers must match" logic and eliminate this variable, like the attached
> patch does on top of this one.

Yes, your version is better. Having a "limit" variable no longer makes
sense (my original patch used "limit" in two places). Feel free to
squeeze it in.

> I couldn't figure out how this would interact with the ignore_ws_change
> codepath, though.  That one shows a clear sign of being bolted on as an
> afterthought (once you fall into that "if()" statement you will not come
> back).

Yes, it does seem bolted on.

I haven't looked much at that if() statement, because I
sort of assumed that because of the return it couldn't do any
harm.

It is too late in my time zone for me to think clearly, but it does
seem that I was wrong and that I'll need to do some changes in
that "if()" statement, and also write some more tests for
the combination of --whitespace=fix and --ignore-space-change.

I'll be back another day.

Thanks for the review.

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB