You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/06/26 19:56:07 UTC
svn commit: r958260 - /subversion/trunk/subversion/libsvn_client/diff.c
Author: dannas
Date: Sat Jun 26 17:56:06 2010
New Revision: 958260
URL: http://svn.apache.org/viewvc?rev=958260&view=rev
Log:
With SVN_EXPERIMENTAL_PATCH defined, only print the git diff header
for deleted paths instead of as previously adding all the hunks that
have been deleted. We *know* from the diff header that the file should
be deleted.
* subversion/libsvn_client/diff.c
(diff_content_changed): If a delete, we're done after printing
the git diff header.
Modified:
subversion/trunk/subversion/libsvn_client/diff.c
Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=958260&r1=958259&r2=958260&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Sat Jun 26 17:56:06 2010
@@ -705,10 +705,11 @@ diff_content_changed(const char *path,
os,
diff_cmd_baton->header_encoding,
path, subpool));
+ svn_pool_destroy(subpool);
+
+ /* We only display the git diff header for deletes. */
+ return SVN_NO_ERROR;
- label1 = diff_label(apr_psprintf(subpool, "a/%s", path1), rev1,
- subpool);
- label2 = diff_label("/dev/null", rev2, subpool);
}
else if (operation == svn_diff_op_added)
{
Re: svn commit: r958260 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Mon, Jun 28, 2010 at 02:38:26PM +0200, Daniel Näslund wrote:
> Since r958537, there is a diff extension switch to enable git diffs:
>
> svn diff -x --git-diff [-g]
>
> In hindsight, I'm not sure that was the best thing to do. I read this
> part from svn help diff:
>
> [[[
> -x --extensions ARG : Default: '-u'. When Subversion is invoking an
> external diff program, ARG is simply passed
> along to the program. But when Subversion
> is using its default internal diff
> implementation, or when Subversion is
> displaying blame annotations, ARG could be
> any of the following:
> ]]]
>
> I thought that --extensions would just extend the diff format and could
> be usable only from within the internal diff application (we don't pass
> on the information needed for creating git diffs to the external diff
> tool) but the svn book says:
>
> [[[
> --extensions (-x) ARGS
>
> Specifies an argument or arguments that Subversion should pass to an
> external diff command when providing differences between files. If you
> wish to pass multiple arguments, you must enclose all of them in quotes
> (for example, svn diff --diff-cmd /usr/bin/diff -x "-b -E"). This switch
> can only be used if you also pass the --diff-cmd switch.
> ]]]
Reverted r958537. I will use a regular command line switch instead.
Daniel
Re: svn commit: r958260 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Mon, Jun 28, 2010 at 02:38:26PM +0200, Daniel Näslund wrote:
> Since r958537, there is a diff extension switch to enable git diffs:
>
> svn diff -x --git-diff [-g]
>
> In hindsight, I'm not sure that was the best thing to do. I read this
> part from svn help diff:
>
> [[[
> -x --extensions ARG : Default: '-u'. When Subversion is invoking an
> external diff program, ARG is simply passed
> along to the program. But when Subversion
> is using its default internal diff
> implementation, or when Subversion is
> displaying blame annotations, ARG could be
> any of the following:
> ]]]
>
> I thought that --extensions would just extend the diff format and could
> be usable only from within the internal diff application (we don't pass
> on the information needed for creating git diffs to the external diff
> tool) but the svn book says:
>
> [[[
> --extensions (-x) ARGS
>
> Specifies an argument or arguments that Subversion should pass to an
> external diff command when providing differences between files. If you
> wish to pass multiple arguments, you must enclose all of them in quotes
> (for example, svn diff --diff-cmd /usr/bin/diff -x "-b -E"). This switch
> can only be used if you also pass the --diff-cmd switch.
> ]]]
Reverted r958537. I will use a regular command line switch instead.
Daniel
Re: svn commit: r958260 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Sat, Jun 26, 2010 at 04:47:46PM -0400, Greg Stein wrote:
> On Sat, Jun 26, 2010 at 13:56, <da...@apache.org> wrote:
> > Author: dannas
> > Date: Sat Jun 26 17:56:06 2010
> > New Revision: 958260
> >
> > URL: http://svn.apache.org/viewvc?rev=958260&view=rev
> > Log:
> > With SVN_EXPERIMENTAL_PATCH defined, only print the git diff header
>
> I still don't understand why we have SVN_EXPERIMENTAL_PATCH. The whole
> patch feature is in-progress. What "funny business" needs to be
> protected? There is no legacy "right way" where changes need to be
> excluded.
The original thought was that we didn't need a commandline switch since
the extra header lines would be considered noise by non-git aware patch
applications. I thought that I would use the ifdef until the diff code
had reached a stable state and then I could remove them and alter the
test output of all the diff tests.
> If you're altering the *diff* output, to enable some Git diff headers,
> then please add a (cmdline) option to do that. The default can't
> change for compat reasons, so the option will be required.
If we use the git format, then deletes, copies and moves may not contain
more than the header lines. In that case, patch applications that could
process svn diff output for 1.6 would not be able to do it for svn
<version with git diffs>.
> If this #define is protecting something else, then ... what? and why
> would PATCH be in the name?
SVN_EXPERIMENTAL_PATCH since I expected to do some changes in the svn
patch code too using ifdefs. I could have used SVN_EXPERIMENTAL_DIFF
instead. I justed wanted one define to turn on to enable the feature.
Since r958537, there is a diff extension switch to enable git diffs:
svn diff -x --git-diff [-g]
In hindsight, I'm not sure that was the best thing to do. I read this
part from svn help diff:
[[[
-x --extensions ARG : Default: '-u'. When Subversion is invoking an
external diff program, ARG is simply passed
along to the program. But when Subversion
is using its default internal diff
implementation, or when Subversion is
displaying blame annotations, ARG could be
any of the following:
]]]
I thought that --extensions would just extend the diff format and could
be usable only from within the internal diff application (we don't pass
on the information needed for creating git diffs to the external diff
tool) but the svn book says:
[[[
--extensions (-x) ARGS
Specifies an argument or arguments that Subversion should pass to an
external diff command when providing differences between files. If you
wish to pass multiple arguments, you must enclose all of them in quotes
(for example, svn diff --diff-cmd /usr/bin/diff -x "-b -E"). This switch
can only be used if you also pass the --diff-cmd switch.
]]]
Thanks,
Daniel
Re: svn commit: r958260 - /subversion/trunk/subversion/libsvn_client/diff.c
Posted by Greg Stein <gs...@gmail.com>.
On Sat, Jun 26, 2010 at 13:56, <da...@apache.org> wrote:
> Author: dannas
> Date: Sat Jun 26 17:56:06 2010
> New Revision: 958260
>
> URL: http://svn.apache.org/viewvc?rev=958260&view=rev
> Log:
> With SVN_EXPERIMENTAL_PATCH defined, only print the git diff header
I still don't understand why we have SVN_EXPERIMENTAL_PATCH. The whole
patch feature is in-progress. What "funny business" needs to be
protected? There is no legacy "right way" where changes need to be
excluded.
If you're altering the *diff* output, to enable some Git diff headers,
then please add a (cmdline) option to do that. The default can't
change for compat reasons, so the option will be required.
If this #define is protecting something else, then ... what? and why
would PATCH be in the name?
>...
Cheers,
-g