Hi,
I hope I've resolved most issues, here are ones I need to ask about:
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1484305)
+++ subversion/include/svn_client.h (working copy)
@@ -2988,8 +2988,8 @@ svn_client_blame(const char *path_or_url,
*
* @a invoke_diff_cmd is used to call an external diff program but may
* not be @c NULL. The command line invocation will override the
- * invoke-diff-cmd invocation entry(if any) in the Subversion
- * configuration file.
+ * invoke-diff-cmd invocation entry(if any) in @c ctx->config.
+ * ### "May not be NULL" !? log-cmd.c and deprecated.c pass NULL for it.
Hmm, what I was trying to communicate was that if a NULL(or "") is passed,
this is an error that will be caught. I'm not quite sure what to write
here now.
The deprecated function is guaranteed to not have an execution path to
invoke-diff-cmd but the log-cmd.c has been fixed.
---------- **** ------------
--- subversion/libsvn_subr/config_file.c (revision 1484305)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -1086,6 +1086,9 @@ svn_config_ensure(const char *config_dir, apr_pool
"# diff3-has-program-arg = [yes | no]" NL
"### Set invoke-diff-cmd to the absolute path of your 'diff'" NL
"### program." NL
+ /* ### Document the replaceables */
(done)
+ /* ### Document how the setting may contain argv too,
+ not only argv[0] */
"### This will override the compile-time default, which is to use" NL
"### Subversion's internal diff implementation." NL
"# invoke-diff-cmd = \"diff -y --label %l1% %f1% --label %l2% %f2%\""NL
Index: subversion/libsvn_subr/io.c
+ /* ### Document how the setting may contain argv too,
+ not only argv[0] */
Not sure what you mean here?
---------- **** ------------
+ /* This reimplements "split a string into argv". Is there an existing
+ svn/apr function that does that can be reused here? (We might gain
+ an "escape spaces" functionality this way.) */
tmp = svn_cstring_split(cmd," ",TRUE, subpool);
I didn't find one, but perhaps I missed it?
---------- **** ------------
+ How does this parse "%%%f1%"? Is "%%f1%%" an error?
%%%f1% becomes %%f1% and %%f1%% becomes %f1%%, neither is an error.
However, %f1%% is parsed out as sub%, also, +%f1% ends up as +sub.
What you cannot do is: %%f1% to get %sub, it will render as %f1%.
If this is a show stopper, we could go back to the triple dash model
and not deal with escaping %'s, or choose another delimiter.
+ (The answers to both of these questions should have been
+ decided prior to coding, and I recall a list thread but I
+ don't recall that thread reached consensus on a specific
+ escaping UI.) Has consensus on the UI implemented here
+ been reached? */
I'm not sure, what do others think?
---------- **** ------------
@@ -3025,6 +3040,7 @@ svn_io_run_external_diff(const char *dir,
if (pexitcode == NULL)
pexitcode = &exitcode;
+ /* if invoke_diff_cmd is "", cmd[0] would be NULL? */
SVN_ERR(svn_io_run_cmd(dir, cmd[0], cmd, pexitcode, NULL, TRUE,
NULL, outfile, errfile, pool));
I check for this condition before it reaches there. Neither "" nor
NULL value for invoke_diff_cmd is accepted, but an error is raised
before it reaches svn_io_create_custom_diff_cmd().
Received on 2013-06-04 10:57:35 CEST