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
Received on 2010-04-14 21:28:31 CEST