[PATCH] filemerge: wrap quotes around tool path

Steve Borho steve at borho.org
Mon Feb 4 16:15:46 CST 2008


On Mon, 2008-02-04 at 15:13 -0600, Matt Mackall wrote:
> On Mon, 2008-02-04 at 15:01 -0600, Steve Borho wrote:
> > On Mon, 2008-02-04 at 14:28 -0600, Matt Mackall wrote:
> > > On Mon, 2008-02-04 at 18:15 -0200, Alexis S. L. Carvalho wrote:
> > > > Thus spake Matt Mackall:
> > > > > 
> > > > > On Mon, 2008-02-04 at 12:33 -0600, Steve Borho wrote:
> > > > > > # HG changeset patch
> > > > > > # User Steve Borho <steve at borho.org>
> > > > > > # Date 1202147114 21600
> > > > > > # Node ID 1da25064cd8eb0278fd3414b78a16a0f8729cf96
> > > > > > # Parent  288ec2f6faa2a362dafa8da81bd03d96e36be5ee
> > > > > > filemerge: wrap quotes around tool path
> > > > > > 
> > > > > > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> > > > > > --- a/mercurial/filemerge.py
> > > > > > +++ b/mercurial/filemerge.py
> > > > > > @@ -184,7 +184,7 @@
> > > > > >          replace = dict(local=a, base=b, other=c, output=out)
> > > > > >          args = re.sub("\$(local|base|other|output)",
> > > > > >                        lambda x: '"%s"' % replace[x.group()[1:]], args)
> > > > > > -        r = util.system(toolpath + ' ' + args, cwd=repo.root, environ=env)
> > > > > > +        r = util.system('"'+toolpath+'" '+args, cwd=repo.root, environ=env)
> > > > > >  
> > > > > >      if not r and _toolbool(ui, tool, "checkconflicts"):
> > > > > >          if re.match("^(<<<<<<< .*|=======|>>>>>>> .*)$", fcm.data()):
> > > > > 
> > > > > Hmm, that breaks things like:
> > > > > 
> > > > > HGMERGE=script -with -some -other -args
> > > > > 
> > > > > We were using precisely that trick in run-tests.py until yesterday, so I
> > > > > suspect other people are using it too.
> > > > 
> > > > We're still using it in some tests:
> > > > 
> > > > $ grep HGMERGE=\" *
> > > > test-filebranch:HGMERGE="python ../merge"; export HGMERGE
> > > > test-merge-symlinks:HGMERGE="python ../echo.py" hg merge
> > > > test-merge-symlinks:HGMERGE="python ../echo.py" hg up 3
> > > > test-merge1:HGMERGE="python ../merge"; export HGMERGE
> > > > test-merge6:HGMERGE="python ../merge"; export HGMERGE
> > > > test-rename-merge2:HGMERGE="python ../merge"; export HGMERGE
> > > > 
> > > > And AFAICS they only work because the script happens to be in another
> > > > directory - we call util.find_exe on the value of $HGMERGE to see if the
> > > > tool is around; that function sees an os.sep in the string and assumes
> > > > it's a full path.
> > > 
> > > Hmm, indeed. We should instead always call HGMERGE exactly as set. Doing
> > > this right probably means having _picktool return (tool, toolpath). That
> > > has the advantage of only calling _findtool once, which is nice.
> > 
> > Yep. I agree.
> > 
> > > > [ui]
> > > > merge = command -arg
> > > 
> > > But this case is a little different. We can treat this a couple ways:
> > > 
> > > a) make ui.merge always take precedence over tools from merge-tools,
> > > regardless of symlink/binary/gui/available tests
> > 
> > For users of binary packages that install merge tool descriptions in the
> > system wide hgrc, being able to pick their tool of choice via ui.merge
> > is handy.  ui.merge definitions with arguments in them will not match
> > anything in merge-tools anyway.
> 
> It's a bit unfortunate when I set my default tool to kdiff3, then try to
> merge without a GUI and it tries kdiff3 even though we *know* it's not
> going to work though. So I think ui.merge should merely be "the highest
> priority tool" (as it is now).

I see what you you mean.  Since ui.merge needs to through the 'check'
logic, it doesn't seem feasible to allow arbitrary arguments to be
specified there unless we want to resort to shlex.split() and only pass
arg[0] to _find_tool().

-- 
Steve Borho (steve at borho.org)
http://www.borho.org/~steve/steve.asc
Key fingerprint = 2D08 E7CF B624 624C DE1F  E2E4 B0C2 5292 F2C6 2C8C



More information about the Mercurial-devel mailing list