[PATCH STABLE] extdiff: quote only options specified at runtime (issue4463)

[FUJIWARA Katsunori]

# HG changeset patch
# User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
# Date 1417454278 -32400
#      Tue Dec 02 02:17:58 2014 +0900
# Branch stable
# Node ID d887d27304009d1cac05a1db4399f69d1778c5f0
# Parent  edf29f9c15f0f171847f4c7a8184cca4e95c8b31
extdiff: quote only options specified at runtime (issue4463)

For arguments and options including white spaces, changeset
72a89cf86fcd introduced fully quoting on all command line arguments
and options for external diff command.

But this causes unexpected behavior of extdiff with WinMerge, because
WinMerge can't work correctly, when command line options in Windows
standard style are quoted: for example, 'WinMerge /r ....' is OK, but
'WinMerge "/r" ....' is NG).

"contrib/mergetools.hgrc" file also specifies some options in Windows
standard style for WinMerge.

See also https://bitbucket.org/tortoisehg/thg/issue/3978/ for detail
about this problem.

On the other hand, users can't specify command line arguments and
options including white spaces, without such quoting in extdiff.

To resolve both issues, this patch introduces mixed quoting policy
"quote only options specified at runtime".

Command line arguments and options specified at runtime by "-o" of
extdiff should be quoted in extdiff, because quoting in the command
line is stripped by the command shell. This stripping breaks arguments
and options including white spaces, if they aren't quoted in extdiff.

On the other hand, command line arguments and options specified in
configuration files can be quoted arbitrarily by users. Such quoting
is still kept at "util.system()" invocation.

BTW, this patch may save the external diff configurations using
command separation (e.g. ';'/'&&'/'||' on POSIX, '&'/'&&'/'||' on
Windows), because:

  - "shlex.split" doesn't suppose them

    "echo foo;echo bar" is splitted into ["echo", "foo;echo", "bar"],
    for example.

  - quoting by "util.shellquote" prevents such separators from working
    correctly

diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -64,7 +64,7 @@ pretty fast (at least faster than having
 from mercurial.i18n import _
 from mercurial.node import short, nullid
 from mercurial import cmdutil, scmutil, util, commands, encoding
-import os, shlex, shutil, tempfile, re
+import os, shlex, shutil, tempfile, re, cStringIO
 
 cmdtable = {}
 command = cmdutil.command(cmdtable)
@@ -121,7 +121,7 @@ def dodiff(ui, repo, diffcmd, diffopts, 
 
     revs = opts.get('rev')
     change = opts.get('change')
-    args = ' '.join(map(util.shellquote, diffopts))
+    args = ' '.join(diffopts)
     do3way = '$parent2' in args
 
     if revs and change:
@@ -268,26 +268,73 @@ def extdiff(ui, repo, *pats, **opts):
     revisions are specified, the working directory files are compared
     to its parent.'''
     program = opts.get('program')
-    option = opts.get('option')
+    # they should be quoted explicitly, because quotations
+    # for them are already stripped by shell
+    option = map(util.shellquote, opts['option'])
     if not program:
         program = 'diff'
         option = option or ['-Npru']
     return dodiff(ui, repo, program, option, pats, opts)
 
+def _splitcmdline(cmdline):
+    '''Split diff command line configuration into command path and options
+
+    This returns ``(commandpath, options)`` tuple. ``options`` is a
+    list, but each command line options specified in ``cmdline`` are
+    not splitted. So, ``len(options)`` is 0 or 1.
+
+    This requires explicit quoting for command path and options
+    including white spaces in configuration files. But on the other
+    hand, users can control quoting for command path and options by
+    themselves.
+
+    >>> _splitcmdline('foo')
+    ('foo', [])
+    >>> _splitcmdline('foo bar baz')
+    ('foo', ['bar baz'])
+    >>> _splitcmdline('"foo foo"')
+    ('foo foo', [])
+    >>> _splitcmdline('"foo foo" bar baz')
+    ('foo foo', ['bar baz'])
+    >>> _splitcmdline('"foo foo" "bar" baz')
+    ('foo foo', ['"bar" baz'])
+    >>> _splitcmdline('foo "bar" baz')
+    ('foo', ['"bar" baz'])
+    >>> _splitcmdline('"foo"/"foo" "bar" baz')
+    ('foo/foo', ['"bar" baz'])
+    '''
+    stream = cStringIO.StringIO(cmdline)
+    # according to "shlex.split" implementation, ``posix`` is True
+    # even on Windows
+    lex = shlex.shlex(stream, posix=True)
+    lex.whitespace_split = True
+    lex.commenters = ''
+
+    cmdpath = lex.next()
+    # current position of stream should be next of the first white
+    # space after diff command path, because characters in stream are
+    # read one by one via "StringIO.read(1)"
+    diffopts = cmdline[stream.tell():]
+
+    if diffopts:
+        return (cmdpath, [diffopts])
+    else:
+        return (cmdpath, [])
+
 def uisetup(ui):
     for cmd, path in ui.configitems('extdiff'):
         if cmd.startswith('cmd.'):
             cmd = cmd[4:]
             if not path:
                 path = cmd
-            diffopts = shlex.split(ui.config('extdiff', 'opts.' + cmd, ''))
+            diffopts = ui.config('extdiff', 'opts.' + cmd, '')
+            diffopts = diffopts and [diffopts] or []
         elif cmd.startswith('opts.'):
             continue
         else:
             # command = path opts
             if path:
-                diffopts = shlex.split(path)
-                path = diffopts.pop(0)
+                path, diffopts = _splitcmdline(path)
             else:
                 path, diffopts = cmd, []
         # look for diff arguments in [diff-tools] then [merge-tools]
@@ -295,11 +342,14 @@ def uisetup(ui):
             args = ui.config('diff-tools', cmd+'.diffargs') or \
                    ui.config('merge-tools', cmd+'.diffargs')
             if args:
-                diffopts = shlex.split(args)
+                diffopts = [args]
         def save(cmd, path, diffopts):
             '''use closure to save diff command to use'''
             def mydiff(ui, repo, *pats, **opts):
-                return dodiff(ui, repo, path, diffopts + opts['option'],
+                # they should be quoted explicitly, because quotations
+                # for them are already stripped by shell
+                runtimeopts = map(util.shellquote, opts['option'])
+                return dodiff(ui, repo, path, diffopts + runtimeopts,
                               pats, opts)
             doc = _('''\
 use %(path)s to diff repository (or selected files)
diff --git a/tests/test-doctest.py b/tests/test-doctest.py
--- a/tests/test-doctest.py
+++ b/tests/test-doctest.py
@@ -31,4 +31,5 @@ testmod('mercurial.util', testtarget='pl
 testmod('hgext.convert.cvsps')
 testmod('hgext.convert.filemap')
 testmod('hgext.convert.subversion')
+testmod('hgext.extdiff')
 testmod('hgext.mq')