2010/2/24 Junio C Hamano <gitster@xxxxxxxxx>: > 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 -- 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