Vladimir Prus <ghost@cs.msu.su> writes:
> It looks like the problem is that diff command is not prepared to
> handle difference only in file properties, without any
> textdelta. The patch below seems to fix the problem and doesn't
> breaks any existing diff tests.
> Shall I sumbit log message and tests for this case, or the way I
> deal with the problem is not the right one (maybe, it would be more
> pure to have a flag telling if a file should be diffed, instead of
> using b->path_end_revision for that?)
Vladirmir, thanks for your patch. Yes, you should submit a log
message and tests for the case -- regardless of whether or not the way
you deal with the problem is the right one! Technically, we don't
require tests to accompany a patch, but I can't think of a better way
to impress a development community than to say, "Here's a bug fix...and
here's my proof!"
As for whether or not a custom flag is needed, whoever reviews your
soon-to-be-submitted, updated patch + log (+ test? :-) :-) ) can
decide that for certain -- I haven't examined the context of the
change yet myself.
Some nits below:
> Index: ./subversion/libsvn_client/repos_diff.c
> ===================================================================
> --- ./subversion/libsvn_client/repos_diff.c
> +++ ./subversion/libsvn_client/repos_diff.c Thu Jan 17 12:04:20 2002
> @@ -568,7 +568,10 @@
> {
> struct file_baton *b = file_baton;
>
> - SVN_ERR (run_diff_cmd (b));
> + // It is possible to have only change to properties, without texdelta
> + // In this case no difference should be reported and no need to run diff.
>
> + if (b->path_end_revision)
> + SVN_ERR (run_diff_cmd (b));
>
> svn_pool_destroy (b->pool);
Hm. Okay, only one nit. We don't use C++ -style comments. This is C
code, and we expect it to compile with compilers that only speak C
(and not C++). So, /* ... */ is your friend.
Please resubmit with the above change, and a log message. And you
might want to examine the context to determine for yourself if that
custom flag is necessary...that's up to you. And *thank you* in
advance for this effort you're making to improve Subversion.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:56 2006