[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r919460 - filtering svn patch targets

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Tue, 30 Mar 2010 12:02:14 +0100

On Mon, 2010-03-29 at 19:42 +0200, Stefan Küng wrote:
> On 29.03.2010 15:08, Julian Foad wrote:
> > On Sat, 2010-03-27, Stefan Küng wrote:
> >> On 27.03.2010 15:23, Stefan Sperling wrote:
> >>> On Fri, Mar 26, 2010 at 07:57:09PM +0100, Stefan Küng wrote:
> >>>> On 26.03.2010 18:17, Stefan Sperling wrote:
> >>>>> Can you check if the current API matches your requirements?
> >>>>> See subversion/tests/libsvn_client/client-test.c for a trivial example.
> >>>>
> >>>> Looks very good! Thanks a lot for implementing this.
> >>>
> >>> Great! You're welcome.
> >>>
> >>>> Another thought, not sure if it would make sense or not:
> >>>> would an option to ignore whitespace changes make sense?
> >>>
> >>> Yes, I guess we could easily make the line-matching ignore whitespace.
> >>> Matching is currently done using a simple strcmp(), so it's fairly naive.
> >>>
> >>> Please file an enhancement issue, with undefined schedule (if someone
> >>> wants to pick this up before 1.7.0, that's fine -- but it's not a
> >>> critical feature).
> >>>
> >>> Ignoring whitespace would help dealing with diffs messed up by e.g. gmail.
> >>> Though I've seen gmail use some magic UTF-8 sequences for whitespace,
> >>> which is really annoying and which a naive approach like ignoring
> >>> single-byte characters like ' ', '\t', etc. may not cope with.
> >>
> >> Filed as issue 3610:
> >> http://subversion.tigris.org/issues/show_bug.cgi?id=3610
> >
> > Can I recommend you add more detail to the request. "Ignore whitespace
> > changes" could be interpreted in many ways, and defining what exactly
> > you mean is a big part of the issue.
> >
> > For example, one change that some mail clients make is to remove a space
> > from the beginning of every line that starts with a space (or starts
> > with two spaces?) so that (using "_" to represent a space):
> >
> > __context-line
> > __context-line
> > __context-line
> > +_plus-line
> > -_minus-line
> > __context-line
> > __context-line
> > __context-line
> >
> > has turned into
> >
> > _context-line
> > _context-line
> > _context-line
> > +_plus-line
> > -_minus-line
> > _context-line
> > _context-line
> > _context-line
> >
> > when the mail is received.
> >
> > If the extension we want is just for that, then
> >
> > "Allow a context line to match a real line that has an extra space at
> > the beginning"
> >
> > is a sufficient definition, whereas
> >
> > "Allow any sequence of whitespace (including an empty line) in the
> > context lines and minus lines to match any sequence of whitespace
> > (including an empty line) in the target file"
> >
> > is a very different requirement.
> >
> > And/or do you want
> >
> > "When a hunk performs only whitespace changes, ignore that hunk"
> >
> > ?
>
> I've updated the issue with more information about what I was thinking
> how it should work.

Thanks.

Can I further recommend that the purpose of the feature should be to try
to match context lines and 'minus' lines against the target file,
ignoring those kinds of white space differences, and to insert 'plus'
lines exactly as they are received in the patch, and *not* try to guess
what combination of white space the 'plus' lines should have. (I am
convinced that any scheme for trying to guess what the 'plus' lines
should look like could only work in a very limited set of cases, and
would make changes that are not wanted in other cases.)

- Julian
Received on 2010-03-30 13:02:58 CEST

This is an archived mail posted to the Subversion Dev mailing list.