[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