Patchwork [3,of,4,V4] windows: quote the specified string only when it has to be quoted

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Dec. 25, 2014, 2:46 p.m.
Message ID <f5178f9404e829694a1a.1419518776@juju>
Download mbox | patch
Permalink /patch/7232/
State Accepted
Commit 1642eb429536cfd240705ad869a7f2e6f3d966f0
Headers show

Comments

Katsunori FUJIWARA - Dec. 25, 2014, 2:46 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1419518006 -32400
#      Thu Dec 25 23:33:26 2014 +0900
# Node ID f5178f9404e829694a1abd1a713f441dac7201e3
# Parent  52b0d3f6ae99ffee8b87e487a34ae41bbc63821d
windows: quote the specified string only when it has to be quoted

Before this patch, "windows.shellquote" (as used as "util.shellquote")
always quotes specified strings with double quotation marks, for
external process invocation.

But some problematic applications can't work correctly, when command
line arguments are quoted: see issue4463 for detail.

On the other hand, quoting itself is needed to specify arguments
containing whitespaces and/or some special characters exactly.

This patch makes "windows.shellquote" examine the specified string and
quote it only when it may have to be quoted for safety.

Patch

diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -148,10 +148,20 @@  def samestat(s1, s2):
 # backslash before every double quote (being careful with the double
 # quote we've appended to the end)
 _quotere = None
+_needsshellquote = None
 def shellquote(s):
     global _quotere
     if _quotere is None:
         _quotere = re.compile(r'(\\*)("|\\$)')
+    global _needsshellquote
+    if _needsshellquote is None:
+        # ":" and "\\" are also treated as "safe character", because
+        # they are used as a part of path name (and the latter doesn't
+        # work as "escape character", like one on posix) on Windows
+        _needsshellquote = re.compile(r'[^a-zA-Z0-9._:/\\-]').search
+    if not _needsshellquote(s) and not _quotere.search(s):
+        # "s" shouldn't have to be quoted
+        return s
     return '"%s"' % _quotere.sub(r'\1\1\\\2', s)
 
 def quotecommand(cmd):
diff --git a/tests/test-extdiff.t b/tests/test-extdiff.t
--- a/tests/test-extdiff.t
+++ b/tests/test-extdiff.t
@@ -107,11 +107,11 @@  issue4463: usage of command line configu
   $ echo a >> a
 #if windows
   $ hg --debug 4463a | grep '^running'
-  running '"echo" a-naked \'single quoted\' "double quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running 'echo a-naked \'single quoted\' "double quoted" *\\a *\\a' in */extdiff.* (glob)
   $ hg --debug 4463b | grep '^running'
-  running 'echo b-naked \'single quoted\' "double quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running 'echo b-naked \'single quoted\' "double quoted" *\\a *\\a' in */extdiff.* (glob)
   $ hg --debug echo | grep '^running'
-  running '"*echo*" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running '*echo* *\\a *\\a' in */extdiff.* (glob)
 #else
   $ hg --debug 4463a | grep '^running'
   running '\'echo\' a-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)
@@ -138,15 +138,15 @@  issue4463: usage of command line configu
   > EOF
 #if windows
   $ hg --debug 4463b2 | grep '^running'
-  running 'echo b2-naked \'single quoted\' "double quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running 'echo b2-naked \'single quoted\' "double quoted" *\\a *\\a' in */extdiff.* (glob)
   $ hg --debug 4463b3 | grep '^running'
-  running 'echo b3-naked \'single quoted\' "double quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running 'echo b3-naked \'single quoted\' "double quoted" *\\a *\\a' in */extdiff.* (glob)
   $ hg --debug 4463b4 | grep '^running'
-  running 'echo "*\\a" "*\\a"' in */extdiff.* (glob)
-  $ hg --debug 4463b4 --option 'being quoted' | grep '^running'
-  running 'echo "being quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
-  $ hg --debug extdiff -p echo --option 'being quoted' | grep '^running'
-  running '"echo" "being quoted" "*\\a" "*\\a"' in */extdiff.* (glob)
+  running 'echo *\\a *\\a' in */extdiff.* (glob)
+  $ hg --debug 4463b4 --option b4-naked --option 'being quoted' | grep '^running'
+  running 'echo b4-naked "being quoted" *\\a *\\a' in */extdiff.* (glob)
+  $ hg --debug extdiff -p echo --option echo-naked --option 'being quoted' | grep '^running'
+  running 'echo echo-naked "being quoted" *\\a *\\a' in */extdiff.* (glob)
 #else
   $ hg --debug 4463b2 | grep '^running'
   running 'echo b2-naked \'single quoted\' "double quoted" \'*/a\' \'$TESTTMP/a/a\'' in */extdiff.* (glob)