[PATCH] extdiff: proof of concept of refactoring to use cmdlines & avoid shlex.split
Mads Kiilerich
mads at kiilerich.com
Mon Dec 8 10:11:43 CST 2014
On 12/08/2014 04:29 PM, FUJIWARA Katsunori wrote:
> These changes seem to become more readable by dividing them into
> patches (1) just integrating "diffcmd" and "diffopts" into "args", (2)
> renaming from "args" to "cmdline", and (3) refactoring comments,
> regular expression, function name and so on (please ignore this
> comment, if these are mixed just for readability of entire PoC).
Yes. It was just a proof of concept, playing around with the code to see
if the idea could work and what cleanups it could make possible.
We still need a real and clean implementation. It would be great if you
would pick it up.
>> --- a/mercurial/posix.py
>> +++ b/mercurial/posix.py
>> @@ -8,7 +8,7 @@
>> from i18n import _
>> import encoding
>> import os, sys, errno, stat, getpass, pwd, grp, socket, tempfile, unicodedata
>> -import fcntl
>> +import fcntl, re
>>
>> posixfile = open
>> normpath = os.path.normpath
>> @@ -312,11 +312,17 @@ if sys.platform == 'cygwin':
>> def checklink(path):
>> return False
>>
>> +_needsshellquote = None
>> def shellquote(s):
>> + global _needsshellquote
>> + if _needsshellquote is None:
>> + _needsshellquote = re.compile(r'[^a-zA-Z0-9._/-]').search
>> if os.sys.platform == 'OpenVMS':
>> return '"%s"' % s
>> + elif _needsshellquote(s):
>> + return "'%s'" % s.replace("'", "'\\''")
>> else:
>> - return "'%s'" % s.replace("'", "'\\''")
>> + return s
>>
>> def quotecommand(cmd):
>> return cmd
> This kind of changes should be applied also on "windows.shellquote",
> shouldn't it ?
>
>
> BTW, in fact, I thought that "posix.shellquote" doesn't have to check
> needs of quoting, because quoting always works correctly on POSIX, at
> first.
Yes, that was just to try to get rid of annoying superfluous quoting in
debug output. That is not directly related to the rest of the PoC with
the approach I ended up using.
/Mads
More information about the Mercurial-devel
mailing list