On Fri, 19 Oct 2007, Stefan Sperling wrote:
> I'm replying here to your reply in the bug tracker
> because the reply got a bit long and I want to resend the diff
> to the list anyway.
> 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.
This makes sense for all three "ignore whitespace"-related options.
> 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.
Yeah, I'm in favor of how your previous patch works. Printing anything
in this case defeats the purpose...
> 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:
> + /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.
I'd say as much, in an inline comment in the test.
+ const char *extensions; /* diff extension args */ /* UTF-8! */
Those two comments could be collapsed into one.
The endline comment on the decl of txn_name could be put after the ";"
+ svn_string_t *str;
This local variable is declared in two separate stack frames, so that we can
format a string and appended the formatted value to "header". Could we dump
the local intermediate var, and use apr_psprintf()/svn_stringbuf_append_cstr()
Received on Mon Oct 22 19:33:47 2007
- application/pgp-signature attachment: stored