[PATCH] New --diff-cmd (just for discussion)

[Gabriela Gibson <ga...@gmail.com>]

Hi,

After looking at Stefan and Ivan's very helpful critique, I had one of
those 'may be good or may be terrible' ideas :)

--invoke-diff-cmd is now absorbed into --diff-cmd (whilst preserving the 
old functionality and look), and most issues that Ivan and Stefan 
flagged up have been resolved thus.

I like this better because there is only one --diff-cmd now, and it's
a much simpler structure (at least this is the theory).

I've adjusted the original diff_external_diffcmd_test and added 4 more, 
and all bar one XFAIL pass.

Because I'm not a great parser coder, and I'm not quite sure that the
current behavior is what we actually want, I've left the library
function style in svn_io__create_custom_diff_cmd() in place for now,
until we have a precise idea of what is actually required, since I
know exactly what this does, which isn't a givens for C strings ;-)

As suggested by danielsh, I attempted to add support for program names 
with a space in them and allowed the user to type 'my\ diff foo bar baz' 
but unfortunately this fails at shell level, and I don't know enough 
about this subject to find a good solution here.

The failed test diff_tests.py 52 shows up like this:

(I renamed the diff_script to 'diff script' at generation time, test
52 is a copy of test 51, bar the diff script name change.)

[[[
g@campion:~/test_trunk/subversion/tests/cmdline$ ./diff_tests.py 52
W: exec of './diff\ script' failed: No such file or 
directorysubversion/svn/diff-cmd.c:427,
W: subversion/libsvn_client/diff.c:2403,
W: subversion/libsvn_client/diff.c:2211,
W: subversion/libsvn_client/diff.c:1676,
W: subversion/libsvn_wc/diff_local.c:500,
W: subversion/libsvn_wc/status.c:2717,
W: subversion/libsvn_wc/status.c:1460,
W: subversion/libsvn_wc/status.c:1115,
W: subversion/libsvn_wc/status.c:834,
W: subversion/libsvn_wc/diff_local.c:369,
W: subversion/libsvn_wc/diff_editor.c:555,
W: subversion/libsvn_client/diff.c:950,
W: subversion/libsvn_client/diff.c:828,
W: subversion/libsvn_subr/io.c:3235: (apr_err=SVN_ERR_EXTERNAL_PROGRAM)
W: svn: E200012: './diff\ script -L "X Y Z" -L "A B C D" %svn_old 
+%svn_new+' was expanded to './diff\ script "X Y Z" -L "A B C D" 
/home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52/.svn/pristine/2c/2c0aa9014a0cd07f01795a333d82485ef6d083e2.svn-base 
+/home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52/iota+ 
' and returned 255
W: CWD: 
/home/g/test_trunk/subversion/tests/cmdline/svn-test-work/working_copies/diff_tests-52
W: EXCEPTION: Failure: Command failed: 
"/home/g/test_trunk/subversion/svn/svn diff --diff-cmd=./diff\ script -L 
"X Y Z" -L "A B C D" %svn_old +%svn_new+ iota ..."; exit code 1
Traceback (most recent call last):
   File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", 
line 1621, in run
     rc = self.pred.run(sandbox)
   File 
"/home/g/test_trunk/subversion/tests/cmdline/svntest/testcase.py", line 
114, in run
     return self._delegate.run(sandbox)
   File 
"/home/g/test_trunk/subversion/tests/cmdline/svntest/testcase.py", line 
176, in run
     return self.func(sandbox)
   File "./diff_tests.py", line 3292, in diff_external_diffcmd_4
     iota_path)
   File 
"/home/g/test_trunk/subversion/tests/cmdline/svntest/actions.py", line 
284, in run_and_verify_svn
     expected_exit, *varargs)
   File 
"/home/g/test_trunk/subversion/tests/cmdline/svntest/actions.py", line 
322, in run_and_verify_svn2
     exit_code, out, err = main.run_svn(want_err, *varargs)
   File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", 
line 702, in run_svn
     *(_with_auth(_with_config_dir(varargs))))
   File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", 
line 368, in run_command
     None, *varargs)
   File "/home/g/test_trunk/subversion/tests/cmdline/svntest/main.py", 
line 560, in run_command_stdin
     '"; exit code ' + str(exit_code))
Failure: Command failed: "/home/g/test_trunk/subversion/svn/svn diff 
--diff-cmd=./diff\ script -L "X Y Z" -L "A B C D" %svn_old +%svn_new+ 
iota ..."; exit code 1
XFAIL: diff_tests.py 52: svn diff --diff-cmd w. spaces in diff program name
]]]

thanks for looking,

Gabriela


[[[

Patch for discussion: Add new syntax facility to --diff-cmd and it's
config counterpart, whilst preserving the old behaviour.

The new syntax allows the use of 4 keywords:
%svn_new_label, %svn_old_label, %svn_old %svn_new

So constructs like:

$svn diff --diff-cmd=
  "diff --ignore-file-name-case -L "One Two" -L %svn_new_label %svn_old 
%svn_new"

are possible.  --extensions is not used if the code does not detect an
old style diff-command command.  Characters immediately adjacent to a 
keyword are also supported (as requested by Julian Foad).

The tests for this patch may be run with ./diff_tests.py 48 through 52

* subversion/include/private/svn_io_private.h
   (svn_io__create_custom_diff_cmd): Declare new function and document
    it.

* subversion/libsvn_subr/io.c

   (svn_io__create_custom_diff_cmd): New Function. This function
     translates user input to diff input.

   (svn_io_run_diff): Refactor function to preserve old behaviour of
     --diff-cmd whilst facilitiating the new extended syntax.

* subversion/tests/cmdline/diff_tests.py

   (diff_external_diffcmd): Change test description, rename the test
     script 'diff' to the more descriptive 'diff_script.

   (diff_external_diffcmd_1): New function.  Test that the --extensions
     option works as before when the --diff-cmd is used with the
     classic syntax.

   (diff_external_diffcmd_2): New function.  Check that the new syntax
     produces a correct, simple diff program call.

   (diff_external_diffcmd_3): New function.  Check that multi word
     labels that may just have one letter are correctly handled.  Check
     that file names can be correctly parsed into +file_name+
     form. This case handles also the cases of -filename or filename+
     where + and - are placeholders for any chosen character.  Check
     that escaped spaces are correctly handled, in case the user has a
     windows-style program name with a space in it.  This currently
     fails at shell level, so this may be the wrong approach
     alltogether.  Error message obtained is provided as a comment at
     the end of the function.

   (diff_external_diffcmd_4): New function with XFAIL result.  Check
     that escaped spaces are correctly handled, in case the user has a
     program name with a space in it.  This currently fails at shell
     level, so this may be the wrong approach alltogether.

   (test_list): Add diff_external_diffcmd_1, diff_external_diffcmd_2,
     diff_external_diffcmd_3 and diff_external_diffcmd_4.

]]]