Re: Branch 'invoke-diff-cmd-feature' is ready for half-way review

On Fri, Oct 18, 2013 at 8:29 PM, Gabriela Gibson
<gabriela.gibson_at_gmail.com> wrote:
...
> 2.) The delimiter:
>
> There appears to be some team miscommunication here -- afaik, the last
> status was that danielsh asked me to implement the semi-colon, so this
> is what you got^Whad :)
>
> Now I had a long think and realised there are other issues with
> expanding that have the potential to be traps, and so, I removed the
> ability to escape the delimiter and changed the syntax to look like
> so:
>
> [[[
> struct replace_tokens_tab
> {
> const char *delimiter;
> const char *replace;
> } tokens_tab[] = { /* Diff terminology */
> { "%svn_new_label%", label1 },
> { "%svn_old_label%", label2 },
> { "%svn_base_label%", label3 },
> { "%svn_old%", from },
> { "%svn_new%", to },
> { "%svn_base%", base },
> { NULL, NULL }
> };
>
> if (label3) /* Merge terminology */
> {
> tokens_tab[0].delimiter = "%svn_to_label%";
> tokens_tab[1].delimiter = "%svn_from_label%";
> tokens_tab[3].delimiter = "%svn_to%";
> tokens_tab[4].delimiter = "%svn_from%";
>
> }
> ]]]
>
> Rationale:
>
> this new syntax frees the commonly used 'fN' and 'lN' variable names and is
> completely unambiguous, and also fairly unique. It's more to type, but much
> less to worry about. Moreover it matches the
> %custom_keyword% syntax for the props, which is new in SVN 1.8:
>
> http://subversion.apache.org/docs/release-notes/1.8#custom-keywords
>
> The issue is that, if we allow escaping (which we need to do if we use
> %f1 %f1% or ;f1 etc) it only is necessary because we're appropriating
> common variable names or a shell character as part of the delimiter.

On Windows, %X% expands to the value of the environment variable X.
I.e. %PATH% yields the value of the PATH variable etc. Best to avoid
it, I think.

On the other hand, %PATH won't be handled specially by the Windows
command shell, so that might be a good option.

Also, the custom-keywords syntax that you referenced above is %r, %u,
%b, ... not %r% etc.

So I think %X is better for your invoke-diff-cmd syntax.

There's one caveat with this syntax on Windows though: it seems you
cannot escape the percent sign at the end of a variable (at least, it
doesn't work on my Windows XP under cmd.exe): if the value of X is
foo, then %X%% will just expand to foo%.

So if you go for the %X syntax, it might not be possible to give the
user a guaranteed way to output

    the_old_label%

as one of the invoke-diff-cmd arguments.

BTW, thanks for continuing your work, and for hanging in there even
though it all takes a while (and sorry for giving the syntax feedback
so late). It would be nice to see this feature ending up in 1.9, if
possible.

-- 
Johan