You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Robert Spier <rs...@pobox.com> on 2003/06/29 08:28:10 UTC
Re: [PATCH] help setting up Visual Diff
cmpilato@collab.net wrote..
> > I wonder why we're passing -u to an anonymous diff program...
> That's a hard-coded input from when we used Gnu diff all the time. It
> should be removed and re-added at runtime by users via the -X
> parameter. Doing this will also allow folks to get the other diff
> types that Gnu diff offers: contextual, side-by-side, etc.
Below find two patches. The first implements the above. The second
makes --diff-cmd "usable".
-R
Patch One:
Log Message:
* io.c: (svn_io_run_diff) remove the obsolete hard coded
arguments passed to the external diff program. The user should
specify any arguments they want with the -x parameter instead.
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 6369)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1154,14 +1154,7 @@
if (user_args != NULL)
nargs += num_user_args;
- else
- nargs += 1; /* -u */
- if (label1 != NULL)
- nargs += 2; /* the -L and the label itself */
- if (label2 != NULL)
- nargs += 2; /* the -L and the label itself */
-
args = apr_palloc (subpool, nargs * sizeof(char *));
i = 0;
@@ -1173,20 +1166,7 @@
for (j = 0; j < num_user_args; ++j)
args[i++] = user_args[j];
}
- else
- args[i++] = "-u"; /* assume -u if the user didn't give us any args */
- if (label1 != NULL)
- {
- args[i++] = "-L";
- args[i++] = label1;
- }
- if (label2 != NULL)
- {
- args[i++] = "-L";
- args[i++] = label2;
- }
-
args[i++] = from;
args[i++] = to;
args[i++] = NULL;
Patch Two:
This is a usability patch. Without this patch using diff-cmd is a
royal pain because of everything that would need to be specified in
-x.
1- search for --diff-cmd programs in the user's path (because this
is a common expectation.)
2- preserve the environment so GUI diff programs (like xxdiff) can
access the ever-important DISPLAY variable, and other things like
HOME. (Currently a completely empty environment is passed. That
can't be right.)
Log Message:
* io.c: (svn_io_run_diff) pass the inherit flag to svn_io_run_cmd
so that external diff commands do not need to be specified with
the full path. (i.e. svn diff configure.in --diff-cmd diff
instead of svn diff configure.in --diff-cmd `which diff`). At a
lower level, this means that the diff command is run with
APR_PROGRAM_PATH instead of APR_PROGRAM, so it is searched for
in the path and the environment is inherited.
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 6369)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1193,7 +1193,7 @@
assert (i == nargs);
- SVN_ERR (svn_io_run_cmd (dir, diff_utf8, args, pexitcode, NULL, FALSE,
+ SVN_ERR (svn_io_run_cmd (dir, diff_utf8, args, pexitcode, NULL, TRUE,
NULL, outfile, errfile, subpool));
/* The man page for (GNU) diff describes the return value as:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Paul L Lussier <pl...@lanminds.com>.
In a message dated: Sun, 29 Jun 2003 01:28:10 PDT
Robert Spier said:
>Below find two patches. The first implements the above. The second
>makes --diff-cmd "usable".
Submitted as Issue 1388:
http://subversion.tigris.org/issues/show_bug.cgi?id=1388
Attempted to add Robert to CC list in IZ but apparently he does not
have a tigris account.
--
Seeya,
Paul
--
Key fingerprint = 1660 FECC 5D21 D286 F853 E808 BB07 9239 53F1 28EE
It may look like I'm just sitting here doing nothing,
but I'm really actively waiting for all my problems to go away.
If you're not having fun, you're not doing it right!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by mark benedetto king <mb...@lowlatency.com>.
On Sun, Jun 29, 2003 at 01:28:10AM -0700, Robert Spier wrote:
> cmpilato@collab.net wrote..
> > > I wonder why we're passing -u to an anonymous diff program...
> > That's a hard-coded input from when we used Gnu diff all the time. It
> > should be removed and re-added at runtime by users via the -X
> > parameter. Doing this will also allow folks to get the other diff
> > types that Gnu diff offers: contextual, side-by-side, etc.
>
> Below find two patches. The first implements the above. The second
> makes --diff-cmd "usable".
Part two committed in revision 6373. Holding off on part one based
on list discussion.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Oliver Geisser <ol...@gmx.de>.
Hi.
Julian Foad wrote:
[...]
>
> diff (GNU diffutils) 2.8.1:
> Options such as --line-format may contain arbitrary characters
> (spaces, quotes, etc.).
> File name arguments may be alone or appended to options like
> "--from-file=".
> Labels may be given after "-L " or "--label=".
>
> pdiff 0.4 (GNU a2ps 4.13)
> Some options may be required to come AFTER the file name arguments.
> Labels cannot be used.
>
> zdiff (gzip 1.3)
> Passes options on to "diff"; generates new temporary file names to
> pass on to "diff" so the "--label=" option is important.
> Cannot handle options that take an argument as a separate word (e.g.
> -L label) but can handle single-word options (e.g. --label=label).
>
Please don't forget the Windows diff tools.
For example:
* WinMerge (http://winmerge.sourceforge.net/)
A free, open-source diff/merge tool based on GNU diff code.
* CSDiff (http://www.componentsoftware.com/products/csdiff/index.htm)
The diff tool from Component Software.
* Araxis Diff (http://www.araxis.com/merge/)
A commercial diff/merge tool which even support 3-way diffs.
In my oppinion the most powerfull is Araxis merge.
The commandline syntax for Araxis merge is:
compare [/? | /h] [/wait] [/aN] [/swap] [/max]
[[/titleN:"<NthFileTitle>"]...] [/2 | /3] <firstFile> <secondFile>
[<thirdFile>] [<mergedOutputFile>]
Ciao, Olli
--og
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Robert Spier <rs...@pobox.com>.
> Especially if the flexibility comes in a flavor like:
> --diff-format="-u -L %l1 %r1 -L %l2 %r2"
> Of course, that's (nearly) exactly the same as just always passing
> four arguments, and letting people write shell script wrappers.
Is there a preference one way or the other? The shell script wrapper
is easier to implement, but will break backward compatibility.
(i.e. --diff-cmd="diff" won't work anymore)
-R
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by mark benedetto king <mb...@lowlatency.com>.
On Mon, Jun 30, 2003 at 02:18:53PM +0100, Julian Foad wrote:
>
> Not futile: the chances are high that enough flexibility to handle the
> first 10 is also enough to handle the other 5.
>
> - Julian
>
Especially if the flexibility comes in a flavor like:
--diff-format="-u -L %l1 %r1 -L %l2 %r2"
Of course, that's (nearly) exactly the same as just always passing
four arguments, and letting people write shell script wrappers.
--ben
`
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Julian Foad <ju...@btopenworld.com>.
Robert Spier wrote:
>
> I removed the -L, because "my pet itch" was to get xxdiff working.
>
> I'd much rather have a diff with incorrect labels, than no diff at
> all.
>
> The current --diff-cmd scheme is broken for everything except
> gnu-diff.
Yes.
>>It would be useful to list the various diff programs that we have
>>access to now, to see what sort of invocation syntax is required for
>>each. This will indicate how much flexibility we need. Here is a
>>start:
>
> xxdiff:
>
> --title1 <arg> Display 'str' instead of filename in filename
> label 1 (left).
>
> --title2 <arg> Display 'str' instead of filename in filename
> label 2 (middle).
>
> But this is probably futile. While we may be able to list 10
> different diff programs we know about, there will be another 5 we
> don't know about.
Not futile: the chances are high that enough flexibility to handle the first 10 is also enough to handle the other 5.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Robert Spier <rs...@pobox.com>.
>
> Robert Spier wrote:
> > Below find two patches. The first implements the above.
>
> You have removed "-u" and the two "-L label" arguments. I agree
> that removing "-u" is a good idea if we are to support arbitrary
> external diff programs; it can easily be passed within "-x" when
> required. However, the "-L" options cannot so easily be passed by
> the user, but they are reasonably important in some circumstances.
> I think we need to devise a better way of invoking an arbitrary diff
> program.
I removed the -L, because "my pet itch" was to get xxdiff working.
While "most" diff programs generally work with <program> <old> <new>
-- all flags are going to be different.
I'd much rather have a diff with incorrect labels, than no diff at
all.
The current --diff-cmd scheme is broken for everything except
gnu-diff.
> Here's another idea: we could specify that the external diff program
> always takes exactly four arguments: the two file names and the two
> labels. The user can provide a simple shell script or similar
> wrapper that accepts those four arguments and invokes the desired
> diff program with an appropriate combination of those arguments and
> other options. This is flexible (in Unix; maybe not in Windows) but
> ugly for the simple cases.
I like the simplicity of this idea. But I think I might want to use a
different argument name, instead of '--diff-cmd'.
svn diff --type xxdiff file1 file2
On Windows this should be relatively simple as well. Batch files or
simple .cmd scripts would be able to handle the needs of the wrapper
scripts.
> It would be useful to list the various diff programs that we have
> access to now, to see what sort of invocation syntax is required for
> each. This will indicate how much flexibility we need. Here is a
> start:
xxdiff:
--title1 <arg> Display 'str' instead of filename in filename
label 1 (left).
--title2 <arg> Display 'str' instead of filename in filename
label 2 (middle).
But this is probably futile. While we may be able to list 10
different diff programs we know about, there will be another 5 we
don't know about.
-R
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Julian Foad <ju...@btopenworld.com>.
> cmpilato@collab.net wrote..
>
>>>I wonder why we're passing -u to an anonymous diff program...
>>
>>That's a hard-coded input from when we used Gnu diff all the time. It
>>should be removed and re-added at runtime by users via the -X
>>parameter.
I agree.
>>Doing this will also allow folks to get the other diff
>>types that Gnu diff offers: contextual, side-by-side, etc.
Actually the default "-u" is not added to the diff command if a (non-empty) "-x" argument is given to svn, so we can already request the other diff types that GNU diff offers. (I consider the "non-empty" restriction to be a bug.)
Robert Spier wrote:
> Below find two patches. The first implements the above.
You have removed "-u" and the two "-L label" arguments. I agree that removing "-u" is a good idea if we are to support arbitrary external diff programs; it can easily be passed within "-x" when required. However, the "-L" options cannot so easily be passed by the user, but they are reasonably important in some circumstances. I think we need to devise a better way of invoking an arbitrary diff program.
For example, we could allow the user to specify a diff command with replaceable parameters for FILE1, FILE2, LABEL1 and LABEL2. Examples:
# To use a simple diff program, not using the human-readable labels:
--diff-cmd="xxdiff $FILE1 $FILE2"
# To recreate the existing default behaviour:
--diff-cmd="diff -u -L $LABEL1 -L $LABEL2 $FILE1 $FILE2"
This is not too bad but quoting and word-splitting can get messy. Actually, the same messiness applies to the present "-x" option - e.g. how can you pass an argument with an embedded space?
Here's another idea: we could specify that the external diff program always takes exactly four arguments: the two file names and the two labels. The user can provide a simple shell script or similar wrapper that accepts those four arguments and invokes the desired diff program with an appropriate combination of those arguments and other options. This is flexible (in Unix; maybe not in Windows) but ugly for the simple cases.
Perhaps we need a combination method that allows simple invocation of programs with simple syntax and flexible invokation of other programs.
It would be useful to list the various diff programs that we have access to now, to see what sort of invocation syntax is required for each. This will indicate how much flexibility we need. Here is a start:
diff (GNU diffutils) 2.8.1:
Options such as --line-format may contain arbitrary characters (spaces, quotes, etc.).
File name arguments may be alone or appended to options like "--from-file=".
Labels may be given after "-L " or "--label=".
pdiff 0.4 (GNU a2ps 4.13)
Some options may be required to come AFTER the file name arguments.
Labels cannot be used.
zdiff (gzip 1.3)
Passes options on to "diff"; generates new temporary file names to pass on to "diff" so the "--label=" option is important.
Cannot handle options that take an argument as a separate word (e.g. -L label) but can handle single-word options (e.g. --label=label).
And of course we should change "GNU diff" to "diff" in the description of "-x" (in subversion/clients/cmdline/main.c).
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] help setting up Visual Diff
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Jun 29, 2003 at 01:28:10AM -0700, Robert Spier wrote:
>...
> +++ subversion/libsvn_subr/io.c (working copy)
> @@ -1154,14 +1154,7 @@
>
> if (user_args != NULL)
> nargs += num_user_args;
> - else
> - nargs += 1; /* -u */
>
> - if (label1 != NULL)
> - nargs += 2; /* the -L and the label itself */
> - if (label2 != NULL)
> - nargs += 2; /* the -L and the label itself */
I don't think that you want to remove the label options. Those given
human-readable information about what is being diff'd. Without them, you
could end up with some *very* unintuitive file names.
The -u should go, tho.
Note: if the diff-cmd can't accept -L, then an intermediate script should
be used to remove them.
> Patch Two:
Oof... please use two emails. I had no idea there was a second patch in
here. I went to delete the bottom of the email from my response, and *then*
I saw a different patch was present.
FWIW, the second one looks good.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org