Re: [PATCHv1bis 1/2] git apply: option to ignore whitespace differences

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> writes:

> Sorry for repying to myself here, but I'm not convinced again. Or to
> be more specific: I think this kind of refactoring is totally out of
> the scope of this patch. So although I agree with you in priciple, if
> you don't mind I'll keep the first two patches simpler and less
> invasive. I'll look into the refactoring as a third step.

I am not interested in applying "this adds a broken ignore-whitespace
option, but as long as you do not use it, it does not hurt the carefully
crafted apply-with-context-whose-ws-breakage-was-fixed codepath."  For
example, I am not convinced at all that your patch does not break the 
update_pre_post_images() nor do I know what text the final pre/postimage
will happen to end up with.  In other words, I do not see a clear logic in
the change.

Also about the broken "only prefix matches" loop, I do not understand why
you would want to consider this preimage from the patch

	"this  has extra whitespace and other stuff\n"

matches the target line

	"this has extra whitespace\n"

only because the prefix matches.

For that matter, I do not think I understand why the leading whitespaces
of s1 and s2 are skipped only when they both begin with a whitespace,
either.

I do not want to be looking at this series until it gets into a bit more
reviewable shape, which I would expect to take at least a week if you are
not working on this full-time (and I presume nobody on the git list is).

Please do not Cc me in the meantime, but please do ask for help from other
people interested in this topic on the list.

--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]