[PATCH 2 of 2 STABLE V2] extdiff: quote only options specified at runtime (issue4463)
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Thu Dec 4 11:22:33 CST 2014
# HG changeset patch
# User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
# Date 1417712035 -32400
# Fri Dec 05 01:53:55 2014 +0900
# Branch stable
# Node ID 4dd202f5daee77c968993cca3e997c2093a79841
# Parent 427cb697d124bea1a7e67ebcdeca2fea81a925c0
extdiff: quote only options specified at runtime (issue4463)
For options including space characters, changeset 72a89cf86fcd
introduced fully quoting on all 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, without such quoting in extdiff, users can't
specify options including space characters.
The root cause of this issue is that "shlex.split + util.shellquote"
combination loses whether users really want to quote each options or
not, even though these can be quoted arbitrarily in configuration
files.
To resolve this problem, this patch introduces mixed quoting policy:
- for options specified at runtime by "-o" of extdiff
These should be quoted in extdiff, because quoting in the command
line is stripped by the command shell. Without quoting in extdiff,
this stripping breaks options including space characters.
This patch explicitly quotes them at "extdiff" and "mydiff",
instead of quoting in "dodiff".
- for options specified in configuration files
To keep quoting in configuration files, this patch uses
"util.shellsplit" instead of "shlex.split" (or avoid "shlex.split"
invocation).
BTW, this patch may save some complicated external diff configurations
like below:
- configuration using command separations and redirections
"shlex.split" doesn't suppose them, and quoting by
"util.shellquote" prevents such separators from working correctly.
For example, "shlex.split" splits "echo foo;echo bar" into
["echo", "foo;echo", "bar"].
- configuration using environment variable containing space characters
Environment variables are expanded before splitting command lines
into each components.
Without quoting, such environment variable containing space
characters is treated as multiple options. But forcible quoting
like 72a89cf86fcd prevents it from being treated as multiple
options.
For example, when CONCAT="foo bar baz':
- mydiff $CONCAT => mydiff foo bar baz (taking 3 arguments)
- mydiff "$CONCAT" => mydiff "foo bar baz" (taking only 1 argument)
This should be decided not by Mercurial implementation but by
users.
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, shutil, tempfile, re
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,7 +268,9 @@ 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']
@@ -280,26 +282,28 @@ def uisetup(ui):
cmd = cmd[4:]
if not path:
path = cmd
- diffopts = shlex.split(ui.config('extdiff', 'opts.' + cmd, ''))
+ diffopts = ui.config('extdiff', 'opts.' + cmd, '')
elif cmd.startswith('opts.'):
continue
else:
# command = path opts
if path:
- diffopts = shlex.split(path)
- path = diffopts.pop(0)
+ path, diffopts = util.shellsplit(path, all=False)
else:
- path, diffopts = cmd, []
+ path, diffopts = cmd, ''
# look for diff arguments in [diff-tools] then [merge-tools]
- if diffopts == []:
- args = ui.config('diff-tools', cmd+'.diffargs') or \
+ if not diffopts:
+ diffopts = ui.config('diff-tools', cmd+'.diffargs') or \
ui.config('merge-tools', cmd+'.diffargs')
- if args:
- diffopts = shlex.split(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'])
+ if diffopts:
+ runtimeopts.insert(0, diffopts)
+ return dodiff(ui, repo, path, runtimeopts,
pats, opts)
doc = _('''\
use %(path)s to diff repository (or selected files)
More information about the Mercurial-devel
mailing list