[PATCH] filemerge: wrap quotes around tool path

[Matt Mackall]

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).

-- 
Mathematics is the supreme nostalgia of our time.