[PATCH] hooks: allow Unix style environment variables on external Windows hooks

[Matt Harbison]

On Mon, 03 Jul 2017 12:15:49 -0400, FUJIWARA Katsunori  
<foozy at lares.dti.ne.jp> wrote:

> At Mon, 3 Jul 2017 22:32:32 +0900,
> Yuya Nishihara wrote:
>>
>> On Sun, 02 Jul 2017 01:45:00 -0400, Matt Harbison wrote:
>> > # HG changeset patch
>> > # User Matt Harbison <matt_harbison at yahoo.com>
>> > # Date 1498969929 14400
>> > #      Sun Jul 02 00:32:09 2017 -0400
>> > # Node ID 5684913c02965f4a6bb43320e8aa189333962dd1
>> > # Parent  61ed2cc98fd17c50c7adf634d777d946781a9fc1
>> > hooks: allow Unix style environment variables on external Windows  
>> hooks
>> >
>> > This will help making common hooks between Windows and non-Windows  
>> platforms.
>> > The hook is launched via cmd.exe, which doesn't understand $var, nor  
>> single
>> > quotes.  Therefore, all that is handled is converting $var to %var%,  
>> and
>> > allowing a literal '$' if escaped with '\'.  Like Unix variables,  
>> Windows will
>> > substitute in the middle of a word (e.g. foo%bar%), so I don't think  
>> there are
>> > any other special cases to handle.
>>
>> (+CC foozy, another Windows expert)
>>
>> Perhaps we'll need to update the help. '$' could be included in a valid
>> filename, which now escape is required.
>>
>> > --- a/mercurial/hook.py
>> > +++ b/mercurial/hook.py
>> > @@ -8,6 +8,7 @@
>> >  from __future__ import absolute_import
>> >
>> >  import os
>> > +import re
>> >  import sys
>> >
>> >  from .i18n import _
>> > @@ -116,6 +117,17 @@
>> >      return r, False
>> >
>> >  def _exthook(ui, repo, htype, name, cmd, args, throw):
>> > +    if pycompat.osname == 'nt':
>> > +        # Replace Unix style environment variables with Windows  
>> style variables,
>> > +        # for replacement by cmd.exe.  Treat '\$' as a literal $.
>> > +        def replacer(m):
>> > +            prefix = m.group(1)
>> > +            if prefix == '\\':
>> > +                return '$' + m.group(2)
>> > +            return prefix + '%' + m.group(2) + '%'
>> > +
>> > +        cmd = re.sub(br'(.?)\$(.+?)\b', replacer, cmd)
>
> How about '(.?)\$([A-Za-z_]\w*)' for more strict matching against
> variable name?

Unfortunately, that strictness is... optimistic:

https://ss64.com/nt/syntax-variables.html

Names can include spaces, '{', '}', and '$'.  Ugh.

Or are you saying we don't need to support those characters, because they  
wouldn't be portable anyway?  (The fact that there are built-ins with some  
non portable characters like 'COMMONPROGRAMFILES(X86)' gives me slight  
pause, though I've never seen any of the other characters in practice.   
And mostly the convenience is when using variables exported by hg.)

> BTW, is supporting simple "{}" surround over engineering in this case?

Seems easy to do, and reasonable.

>     def replacer(m):
>         prefix = m.group(1)
>         if prefix == '\\':
>             return '$' + m.group(2)
>         varname = m.group(2)
>         if varname.startswith('{'):
>             return prefix + '%' + m.group(2)[1:-1] + '%'
>         return prefix + '%' + m.group(2) + '%'
>
>     cmd = re.sub(br'(.?)\$({[A-Za-z_]\w*}|[A-Za-z_]\w*)', replacer, cmd)
>
> This allows concatenation of environment variables and normal texts
> (e.g. ${FOO}BAR).
>
>> Can you move this to windows.py, which hosts several shell-related  
>> functions.
>> I think the function can have some doctests.
>>