Patchwork filemerge: pass a default value to _toolstr

login
register
mail settings
Submitter Kostia Balytskyi
Date Oct. 26, 2017, 6:07 p.m.
Message ID <81db944aae69a96175fd.1509041239@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/25287/
State Accepted
Headers show

Comments

Kostia Balytskyi - Oct. 26, 2017, 6:07 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1509041226 25200
#      Thu Oct 26 11:07:06 2017 -0700
# Branch stable
# Node ID 81db944aae69a96175fd0d917834e67534a92735
# Parent  9e9ff559440e4733fda49ddeb1b9ce5e7c9660af
filemerge: pass a default value to _toolstr

After a refactoring, _toolstr stopped having default="" as one of it's args,
therefore when called without a default it returns None and not "". This causes
concatenation to fail.
Yuya Nishihara - Oct. 28, 2017, 5:19 a.m.
On Thu, 26 Oct 2017 11:07:19 -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1509041226 25200
> #      Thu Oct 26 11:07:06 2017 -0700
> # Branch stable
> # Node ID 81db944aae69a96175fd0d917834e67534a92735
> # Parent  9e9ff559440e4733fda49ddeb1b9ce5e7c9660af
> filemerge: pass a default value to _toolstr

(issue5718)

> After a refactoring, _toolstr stopped having default="" as one of it's args,
> therefore when called without a default it returns None and not "". This causes
> concatenation to fail.
> 
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -116,7 +116,7 @@ def findexternaltool(ui, tool):
>              continue
>          p = util.lookupreg(k, _toolstr(ui, tool, "regname"))
>          if p:
> -            p = util.findexe(p + _toolstr(ui, tool, "regappend"))
> +            p = util.findexe(p + _toolstr(ui, tool, "regappend", ""))

Perhaps this has to be added to the config table instead.
Augie Fackler - Oct. 31, 2017, 2:37 a.m.
On Sat, Oct 28, 2017 at 02:19:17PM +0900, Yuya Nishihara wrote:
> On Thu, 26 Oct 2017 11:07:19 -0700, Kostia Balytskyi wrote:
> > # HG changeset patch
> > # User Kostia Balytskyi <ikostia@fb.com>
> > # Date 1509041226 25200
> > #      Thu Oct 26 11:07:06 2017 -0700
> > # Branch stable
> > # Node ID 81db944aae69a96175fd0d917834e67534a92735
> > # Parent  9e9ff559440e4733fda49ddeb1b9ce5e7c9660af
> > filemerge: pass a default value to _toolstr
>
> (issue5718)

Queued with that added, thanks.

>
> > After a refactoring, _toolstr stopped having default="" as one of it's args,
> > therefore when called without a default it returns None and not "". This causes
> > concatenation to fail.
> >
> > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> > --- a/mercurial/filemerge.py
> > +++ b/mercurial/filemerge.py
> > @@ -116,7 +116,7 @@ def findexternaltool(ui, tool):
> >              continue
> >          p = util.lookupreg(k, _toolstr(ui, tool, "regname"))
> >          if p:
> > -            p = util.findexe(p + _toolstr(ui, tool, "regappend"))
> > +            p = util.findexe(p + _toolstr(ui, tool, "regappend", ""))
>
> Perhaps this has to be added to the config table instead.

Very likely. I'm not going to sweat it for right now, as this fixes a
regression in any case.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -116,7 +116,7 @@  def findexternaltool(ui, tool):
             continue
         p = util.lookupreg(k, _toolstr(ui, tool, "regname"))
         if p:
-            p = util.findexe(p + _toolstr(ui, tool, "regappend"))
+            p = util.findexe(p + _toolstr(ui, tool, "regappend", ""))
             if p:
                 return p
     exe = _toolstr(ui, tool, "executable", tool)