[PATCH] Update: svnlook diff and whitespace

Hello Michael,

I'm replying here to your reply in the bug tracker
(http://subversion.tigris.org/issues/show_bug.cgi?id=2912)
bcause the reply got a bit long and I want to resend the diff
to the list anyway.

I will add a link to this mail in the bug tracker once it
has reached the archives.

1.) Regarding the diff to main.c:

My diff assumes that the user simply does not care about files
that contain whitespace-only changes and does not want any information
printed for them, not even the filename.

So the idea is that there is no output at all if there are only
whitespace changes in a file and --ignore-whitespace is specified.

This behaviour is consistent with how 'svn diff' is behaving.

So in this case, the hunk you proposed alone is not sufficient
because we need to delay printing the header to stdout until we know
whether there really is a diff to print.

I don't know if this behaviour is desired, we might as well print
something like "whitespace-only changes ignored" or simply favour
nothing following a diff header. I am open for opinions on that.
But I think we should try to make 'svn diff' and 'svnlook diff'
behave as similarly as possible.

I've created a recipe to illustrace this, and to test the patch.
See attachment.

With the updated patch you sent me in private mail, which
removes the buffering of the diff header, the following happens
when I run the recipe:

  + rm -rf /tmp/svnroot /tmp/wc
  + svnadmin create /tmp/svnroot
  + mkdir /tmp/wc
  + echo a b
  + > /tmp/wc/a
  + svn import -m importing /tmp/wc file:///tmp/svnroot/project
  Adding /tmp/wc/a
  
  Committed revision 1.
  + rm -rf /tmp/wc
  + svn co file:///tmp/svnroot/project /tmp/wc
  A /tmp/wc/a
  Checked out revision 1.
  + echo a b
  + > /tmp/wc/a
  + cd /tmp/wc
  + svn commit -m whitespace change
  Sending a
  Transmitting file data .
  Committed revision 2.
  + /tmp/svnlook-cmpilato/bin/svnlook diff -r2 /tmp/svnroot
  Modified: project/a
  ===================================================================
  --- project/a 2007-10-19 18:24:36 UTC (rev 1)
  +++ project/a 2007-10-19 18:24:37 UTC (rev 2)
  @@ -1 +1 @@
  -a b
  +a b
  
  + /tmp/svnlook-cmpilato/bin/svn diff -c2 file:///tmp/svnroot
  Index: project/a
  ===================================================================
  --- project/a (revision 1)
  +++ project/a (revision 2)
  @@ -1 +1 @@
  -a b
  +a b
  + /tmp/svnlook-cmpilato/bin/svnlook diff -r2 -x --ignore-space-change /tmp/svnroot
  Modified: project/a
  ===================================================================
  
  + /tmp/svnlook-cmpilato/bin/svn diff -c2 -x --ignore-space-change file:///tmp/svnroot
  

Note the diff header being printed with an empty line following it,
and 'svn diff' printing nothing.

With my updated patch (attached) which, like the original,
creates the diff header in a buffer to be able to decide not to
print it, I get this:

  + rm -rf /tmp/svnroot /tmp/wc
  + svnadmin create /tmp/svnroot
  + mkdir /tmp/wc
  + echo a b
  + > /tmp/wc/a
  + svn import -m importing /tmp/wc file:///tmp/svnroot/project
  Adding /tmp/wc/a
  
  Committed revision 1.
  + rm -rf /tmp/wc
  + svn co file:///tmp/svnroot/project /tmp/wc
  A /tmp/wc/a
  Checked out revision 1.
  + echo a b
  + > /tmp/wc/a
  + cd /tmp/wc
  + svn commit -m whitespace change
  Sending a
  Transmitting file data .
  Committed revision 2.
  + /tmp/svnlook-stsp/bin/svnlook diff -r2 /tmp/svnroot
  Modified: project/a
  ===================================================================
  --- project/a 2007-10-19 18:23:14 UTC (rev 1)
  +++ project/a 2007-10-19 18:23:15 UTC (rev 2)
  @@ -1 +1 @@
  -a b
  +a b
  
  + /tmp/svnlook-stsp/bin/svn diff -c2 file:///tmp/svnroot
  Index: project/a
  ===================================================================
  --- project/a (revision 1)
  +++ project/a (revision 2)
  @@ -1 +1 @@
  -a b
  +a b
  + /tmp/svnlook-stsp/bin/svnlook diff -r2 -x --ignore-space-change /tmp/svnroot
  + /tmp/svnlook-stsp/bin/svn diff -c2 -x --ignore-space-change file:///tmp/svnroot

No output from neither svn nor svnlook.

2.) Regarding the test:

See merge_conflict_markers_matching_eol() in merge_tests.py
and conflict_markers_matching_eol() in update_tests.py
They both do the exact same thing, I have copied this piece from
either of these, I'm not sure which exactly.

If my test does this wrong, then these tests do it wrong, too.
The goal of this test is of course to test whether
'svnlook diff --ignore-eolstyle' really works with all possible
eol styles. If there is a better way of doing this, I'd very much
like to know about it.

3.) There is a log message inside the patch now.

-- 
Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner