On Wed, Apr 14, 2010 at 08:18:06PM +0200, Alan Barrett wrote: > On Wed, 14 Apr 2010, Stefan Sperling wrote: > > Yes. Just use whatever comes from the patch, including context lines. > > This is consistent with the current behaviour. I think we should avoid > > special cases where this rule is currently not true anymore. > > (I'm not sure how UNIX patch behaves wrt context lines, actually. > > Might be worth checking.) > > If applying a patch ever causes changes to lines that are marked in the > patch as context lines (not as lines to be removed or added), then I > expect people to be unhappy. The context lines in the patch file are > known or suspected to be damaged (or else the user would not have asked > to ignore whitespace changes), so copying the context lines from the > patch is not wanted. > > The GNU patch implementation that I just tried does the right thing > here: when passed the "--ignore-whitespace" option, it ignores > whitespace changes in context lines and lines marked as being removed, > and it copies context lines from the input file, not from the patch.
Darn GNU patch! :) I guess there's a reason for the maze of if statements that makes up it's source code! It does more than one expects! > I don't buy the argument that it's necessary, for consistency with > current svn behaviour, to copy context lines from the patch to > the output file. Current behaviour does not include any kind of > "ignore whitespace" option, so the context lines in the input file > are guaranteed not to differ from the context lines in the patch, > so an outside observer (without knowledge of the internals of the > implementation) cannot tell whether the current implementation copies > context lines from the patch or from the input file. Changing the > implementation to copy context lines from the input file would therefore > not be an incompatible change. Nope, it wouldn't be an incompatible change. It's just that it would involve rewriting how we construct the streams from the patch and how we apply the hunks. The current behaviour is (writing it down to remind myself): When parsing ------------- Lines beginning with ' ' and '-' are read to original_stream Lines beinning with ' ' and '+' are read to modified_stream When applying --------------- for each hunk in patch determine the line the hunk should be applied at by matching original_stream to target for each hunk in patch copy lines from target until we reach match-point copy lines from modified_stream If we would implement --ignore-whitespaces to have the behaviour you suggested we would have to rewrite the parts dealing with modified_stream to make us able to distinguish between context lines and added lines. At the moment, we filter out the leading characters (' ', '+', '-') before writing to the streams. In the end it boils down to: From my standpoint as a newbie programmer I'm very dense when it comes to any major changes. I do agree that the behaviour you describe is the preferred but I'm a bit annoyed about having to do all those changes for something that is a very small part of the typical use case set. In my world, I would probably just ask the submitter to retransmit the patch using some other mail software. I hope I'm not too grumpy. I just don't want to rush into some bigger change without weighing the options. cheers, Daniel