Gabriela Gibson wrote: > On 19/03/13 13:09, Julian Foad wrote: >> For the record, the summary line of issue #2044 is 'Fully customizable >> external diff invocations'. (I like to mention the summary alongside >> the number as I am not good at memorizing issue numbers.) I'm curious >> about your patch because I am interested in issue #2044 and would like >> to see how this particular change would fit in. >> >> Please could you tell me more precisely what your patch does and why? >> Of course I could read carefully through your patch to discover the >> 'what', but not the 'why'. > > Hi Julian, > > It's not really a patch as such, not yet anyway :> Also, this strictly > speaking is issue 2074, which was marked as a duplicate of 2044 since it > partially solves 2074.
For my and everyone else's benefit, issue #2074 [1] is titled, "--extensions '' doesn't work." and requests a way to get rid of the '-u' and '-L' options that Subversion supplies when invoking an external diff, for use with diff tools that don't support those switches. The tools 'bbdiff' and 'sdiff' are mentioned, on a Mac. > Given the following perl script posing as my diff command: [...] > The output of this 'diff-command' looks like on a test repository with > a single change: [...] > Arg 0 is >-u< > Arg 1 is >-L< > Arg 2 is >testfile (revision 1)< > Arg 3 is >-L< > Arg 4 is >testfile (working copy)< > Arg 5 is >/home/.../.svn/pristine/91/91b7...2d32.svn-base< > Arg 6 is >/home/g/tmp/test-wc/testfile< > The goal of my patch is to provide two facilities: > 1. Allow the complete removal of the "-u" switch. > 2. Allow the replacement of the "-L" switch > > (One question out of 2 above is, should we allow the complete removal > of the -L as the patch does for the "-u". Currently, there is an > empty element in argv, the alternative is to add another flag, (say) > --no-diff-label) See below. > The patch adds a 'char *user_label_string' and a 'svn_boolean_t > ext_string_present' parameter to the internal structures. Never mind the internal structures yet. > This part of the patch (once it's completed) allows you to do: > > [...] svn diff -x "" --diff-label="" > --diff-cmd=/home/g/programming/perlfiles/dump_diff.pl > Index: subversion/include/svn_client.h > =================================================================== > Dumping @ARGV... > Arg 0 is >< > Arg 1 is >subversion/include/svn_client.h (revision 1458417)< > Arg 2 is >< > Arg 3 is >subversion/include/svn_client.h (working copy)< > Arg 4 is >> /home/g/trunk_diff4/.svn/pristine/90/908abcdec38f17df77baf339075101ba4471a4e4.svn-base< > Arg 5 is >/tmp/svn-40JzGW< I can assure you that's not going to fly with empty arguments. Try it with one or two typical diff utilities such as GNU 'diff' and 'kdiff3'. Yes we certainly need to be able to remove the '-L' arguments completely. I would also say we need to be able to remove the labels themselves (args 1 and 3 here) because I'm sure not all diff tools will accept them. > or also: > > [...] svn diff -x "Hansel" --diff-label="Gretel" > --diff-cmd=/home/g/programming/perlfiles/dump_diff.pl > Index: subversion/include/svn_client.h > =================================================================== > Dumping @ARGV... > Arg 0 is >Hansel< > Arg 1 is >Gretel< > Arg 2 is >subversion/include/svn_client.h (revision 1458417)< > Arg 3 is >Gretel< > Arg 4 is >subversion/include/svn_client.h (working copy)< > Arg 5 is >/home/.../.svn/pristine/90/908a...a4e4.svn-base< > Arg 6 is >/tmp/svn-BhDgjc< Might some diff programs want a different option name for each label, such as "--l1 label1 --l2 label2"? I would expect so. From kdiff3's help: --L1 alias1 Visible name replacement for input file 1 (base). --L2 alias2 Visible name replacement for input file 2. --L3 alias3 Visible name replacement for input file 3. -L, --fname alias Alternative visible name replacement. Supply this once for every input. so kdiff3 optionally accepts '--L1' etc., although it has an alternative. That alone makes me suspect there are programs that require options like '--L1'. And do some diff programs require the label to be attached to the option name like "-Llabel1" or "-L:label1" rather than one arg being the option and the next arg being the label? Again, I would expect so. You'll need to check a few typical diff tools (or their user manuals) to answer those kinds of questions. Overall, I think command-line options like these are too specific. We don't want to grow a dozen new command-line options as we discover a dozen different ways that external diff tools can be controlled. Rather, we need to look at this in the context of how to configure a generic diff command more easily. For example, one possibility is we could use some sort of pattern-substitution in a specified template: we could take a pattern argument, in which any text enclosed in angle brackets is a special keyword that Subversion replaces with one of a small fixed set of things, so svn diff --invoke-diff='kdiff3 --L1=<label1> <file1> --L2=<label2> <file2>' might cause the diff tool to be invoked as Arg 0 is >--L1=subversion/include/svn_client.h (revision 1458417)< Arg 1 is >--L2=subversion/include/svn_client.h (working copy)< Arg 2 is >/home/.../...a4e4.svn-base< Arg 3 is >/tmp/svn-BhDgjc< This is nothing more than a made-up example of the sort of thing we could do. For this example, I have assumed there is a rule that if the template doesn't include replaceable parameters '<tmpfile1>' and '<tmpfile2>' then those will be appended automatically. With this kind of template idea, we'd also have to figure out what to do about spaces and quoting, which is a problem that has standard solutions but is not totally trivial. There are surely other, different ways to allow more flexible configuration. Of course, users won't normally want to be typing this configuration on the command line, they'll want it to be in a config file. But it's useful to have the possibility of doing it on the command line as well. Overall, I think we should address the '-u' and '-L' issue as part of a larger issue and not implement a solution that specifically provides just a way to manage those particular options. - Julian [1] <http://subversion.tigris.org/issues/show_bug.cgi?id=2074>
