syntax for --invoke-diff-cmd (was: Branch 'invoke-diff-cmd-feature' is ready for half-way review)

[Johan Corveleyn <jc...@gmail.com>]

[ Changing subject because perhaps some more discussion might arise.
More below. ]

On Mon, Oct 28, 2013 at 11:48 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> On Fri, Oct 18, 2013 at 8:29 PM, Gabriela Gibson
> <ga...@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.

OK, a little googling helped here [1]. It seems the caret (^) can be
used to escape the percent sign:

[[[
C:\>echo %PATH^%
%PATH%
]]]

I wouldn't want to use the caret all the time, so I still think the %X
syntax is slightly better. And if needed, Windows users can still
escape any percent signs that confuse the shell (for instance if they
absolutely want a trailing percent sign after one of your variables).

BTW, it's not just "trailing percent signs" that can become a problem
with the %X syntax, it's also that the "beginning percent sign" of one
variable can be seen by the shell as the trailing percent sign of
whatever came before. Indeed, it seems that environment variables can
have spaces:

[[[
C:\>set A =blah

C:\>echo %A %foo
blahfoo
]]]


But perhaps we shouldn't worry too much about users setting the
environment variable with name 'svn_old ' :-).


[1] http://superuser.com/questions/409546/escaping-s-in-file-folder-names-at-the-command-line

-- 
Johan

Re: syntax for --invoke-diff-cmd (was: Branch 'invoke-diff-cmd-feature' is ready for half-way review)

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

On 28/10/13 23:15, Johan Corveleyn wrote:

Hi Johan and everyone,

substitution variables no longer needs to be escaped, since we
set aside 10 keywords that may not be used for anything else.

The escape mechanism was a hack because I didn't want to
monopolize common variable names(like f1, l1 etc), but Roderich's
timely reminder to use the correct svn syntax of 'old' and 'new'
in the help file made the natural solution obvious: just use the
pattern:

svn_[old|new|mine|yours|base]...*

I also like this approach because it removes the need for the
user to remember whether f1 token was the old or the new, or
perhaps mine, yours or base.

Currently the extra % at the end is there because we strcmp and
then just substitute, even when it's inside a string to allow for
things like + and - in front and at the end of file names, some
diff programs require this syntax and it accommodates input like
foo=%svn_old.

Currently, the % at the end ensures are %svn_old% and
%svn_old_label% are unique tokens and if we remove the end %,
%svn_old_label would end up expanded as /tmp/file_label.

So to make this new %svn-* pattern work, we could change it thus:

%svn_old
%svn_label_old

Ie, just put 'label' in second position, I think this is an
acceptable looking, clear syntax and keeps things simple.

"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). "

Thanks for everyone's patience in that matter and for putting up
with my unusual syntax experiments :)

Picking syntax *is* important because such decisions shape many
hours of user time, and changing things later on is always
unpopular and usually impossible.

So taking the extra time and care is well worth the effort, if
people are not 100% happy with a choice, there is always a good
reason for it.

It's very easy to change the delimiter patterns in the code, so
the question can be considered right up to the moment where we
release, it doesn't hold anything up at all if someone can think
of a better scheme in a few week's time.

Gabriela

Ps,: I'm sure we can get it completed for 1.9 (do we have an ETA here?) 
and if time is getting short and I'm still confuzzled, I'll start asking 
for help.  What takes the time is me learning things and getting lost on 
silly issues like this:

http://gabriela-gibson.blogspot.co.uk/2013/10/gdb-and-shell-game.html