[PATCH] hooks: allow Unix style environment variables on external Windows hooks
Yuya Nishihara
yuya at tcha.org
Wed Jul 5 13:01:20 UTC 2017
On Tue, 04 Jul 2017 23:39:59 -0400, Matt Harbison wrote:
> On Tue, 04 Jul 2017 09:13:46 -0400, Yuya Nishihara <yuya at tcha.org> wrote:
>
> > On Tue, 04 Jul 2017 12:42:08 +0900, FUJIWARA Katsunori wrote:
> >> At Mon, 03 Jul 2017 21:57:01 -0400,
> >> Matt Harbison wrote:
> >> > On Mon, 03 Jul 2017 12:15:49 -0400, FUJIWARA Katsunori
> >> > <foozy at lares.dti.ne.jp> wrote:
> >> > >> > 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.)
> >>
> >> Yes, I think that platform specific characters and features
> >> (substitutions, and so on) can be omitted, because main concern of
> >> this patch is portability of external hook command line between
> >> platforms.
> >
> > FWIW, I found ntpath.expandvars() does support $name and ${name} on
> > Windows.
> >
> > https://docs.python.org/2/library/os.path.html#os.path.expandvars
>
> Thanks, I forgot about that too. I switched to that, and got some
> unexpected failures. I'll have to dig in more, and that should give time
> for stable to merge back into default to pick up the help text changes.
Oops, I wanted to tell you that we could copy the expansion rule (and maybe
a part of the implementation.) Variable expansion should be done only once,
but cmd.exe will do that again.
> BTW, if I end up using this, should it be conditionalized to just
> Windows? (In theory, it shouldn't matter for non-Windows, but just in
> case...)
Just Windows. Unix shell has much more complex syntax.
More information about the Mercurial-devel
mailing list