Patchwork [08,of,11] configitems: register the full 'merge-tools' config and sub-options

login
register
mail settings
Submitter Boris Feld
Date Oct. 13, 2017, 5:55 p.m.
Message ID <6ad9f6f42f33fe5efb78.1507917314@FB>
Download mbox | patch
Permalink /patch/24845/
State Accepted
Headers show

Comments

Boris Feld - Oct. 13, 2017, 5:55 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1507487833 -7200
#      Sun Oct 08 20:37:13 2017 +0200
# Node ID 6ad9f6f42f33fe5efb78f76d39c41f28333144d6
# Parent  0824a3519c14d643a81dd4f718846f15045772ec
# EXP-Topic config.register.ready
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 6ad9f6f42f33
configitems: register the full 'merge-tools' config and sub-options

We register the merge-tools config section (which has an arbitrary base config
value) and the possible sub-attribute. The sub-attribute has to be registered
first or at the same time otherwise the '.*' item would shadow them.
Augie Fackler - Oct. 13, 2017, 7:37 p.m.
On Fri, Oct 13, 2017 at 07:55:14PM +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1507487833 -7200
> #      Sun Oct 08 20:37:13 2017 +0200
> # Node ID 6ad9f6f42f33fe5efb78f76d39c41f28333144d6
> # Parent  0824a3519c14d643a81dd4f718846f15045772ec
> # EXP-Topic config.register.ready
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 6ad9f6f42f33
> configitems: register the full 'merge-tools' config and sub-options
>
> We register the merge-tools config section (which has an arbitrary base config
> value) and the possible sub-attribute. The sub-attribute has to be registered
> first or at the same time otherwise the '.*' item would shadow them.
>
> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> --- a/mercurial/configitems.py
> +++ b/mercurial/configitems.py
> @@ -498,6 +498,60 @@
>  coreconfigitem('merge', 'preferancestor',
>          default=lambda: ['*'],
>  )
> +coreconfigitem('merge-tools', '.*',
> +    default=None,
> +    generic=True,
> +)

Shouldn't this be [^.]*, since a . means a sub-attribute?

> +coreconfigitem('merge-tools', r'.*\.args$',
> +    default="$local $base $other",
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.binary$',
> +    default=False,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.check$',
> +    default=list,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.checkchanged$',
> +    default=False,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.executable$',
> +    default=dynamicdefault,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.fixeol$',
> +    default=False,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.gui$',
> +    default=False,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.priority$',
> +    default=0,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.premerge$',
> +    default=dynamicdefault,
> +    generic=True,
> +    priority=-1,
> +)
> +coreconfigitem('merge-tools', r'.*\.symlink$',
> +    default=False,
> +    generic=True,
> +    priority=-1,
> +)
>  coreconfigitem('pager', 'ignore',
>      default=list,
>  )
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -30,16 +30,14 @@
>      util,
>  )
>
> -def _toolstr(ui, tool, part, default=""):
> -    return ui.config("merge-tools", tool + "." + part, default)
> +def _toolstr(ui, tool, part, *args):
> +    return ui.config("merge-tools", tool + "." + part, *args)
>
> -def _toolbool(ui, tool, part, default=False):
> -    return ui.configbool("merge-tools", tool + "." + part, default)
> +def _toolbool(ui, tool, part,*args):
> +    return ui.configbool("merge-tools", tool + "." + part, *args)
>
> -def _toollist(ui, tool, part, default=None):
> -    if default is None:
> -        default = []
> -    return ui.configlist("merge-tools", tool + "." + part, default)
> +def _toollist(ui, tool, part):
> +    return ui.configlist("merge-tools", tool + "." + part)
>
>  internals = {}
>  # Merge tools to document.
> @@ -187,8 +185,8 @@
>      for k, v in ui.configitems("merge-tools"):
>          t = k.split('.')[0]
>          if t not in tools:
> -            tools[t] = int(_toolstr(ui, t, "priority", "0"))
> -        if _toolbool(ui, t, "disabled", False):
> +            tools[t] = int(_toolstr(ui, t, "priority"))
> +        if _toolbool(ui, t, "disabled"):
>              disabled.add(t)
>      names = tools.keys()
>      tools = sorted([(-p, tool) for tool, p in tools.items()
> @@ -328,7 +326,7 @@
>      try:
>          premerge = _toolbool(ui, tool, "premerge", not binary)
>      except error.ConfigError:
> -        premerge = _toolstr(ui, tool, "premerge").lower()
> +        premerge = _toolstr(ui, tool, "premerge", "").lower()
>          if premerge not in validkeep:
>              _valid = ', '.join(["'" + v + "'" for v in validkeep])
>              raise error.ConfigError(_("%s.premerge not valid "
> @@ -503,7 +501,7 @@
>                 }
>          ui = repo.ui
>
> -        args = _toolstr(ui, tool, "args", '$local $base $other')
> +        args = _toolstr(ui, tool, "args")
>          if "$output" in args:
>              out, a = a, back # read input from backup, write to original
>          replace = {'local': a, 'base': b, 'other': c, 'output': out}
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Boris Feld - Oct. 16, 2017, 2 p.m.
On Fri, 2017-10-13 at 15:37 -0400, Augie Fackler wrote:
> On Fri, Oct 13, 2017 at 07:55:14PM +0200, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1507487833 -7200
> > #      Sun Oct 08 20:37:13 2017 +0200
> > # Node ID 6ad9f6f42f33fe5efb78f76d39c41f28333144d6
> > # Parent  0824a3519c14d643a81dd4f718846f15045772ec
> > # EXP-Topic config.register.ready
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > #              hg pull https://bitbucket.org/octobus/mercurial-deve
> > l/ -r 6ad9f6f42f33
> > configitems: register the full 'merge-tools' config and sub-options
> > 
> > We register the merge-tools config section (which has an arbitrary
> > base config
> > value) and the possible sub-attribute. The sub-attribute has to be
> > registered
> > first or at the same time otherwise the '.*' item would shadow
> > them.
> > 
> > diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> > --- a/mercurial/configitems.py
> > +++ b/mercurial/configitems.py
> > @@ -498,6 +498,60 @@
> >  coreconfigitem('merge', 'preferancestor',
> >          default=lambda: ['*'],
> >  )
> > +coreconfigitem('merge-tools', '.*',
> > +    default=None,
> > +    generic=True,
> > +)
> 
> Shouldn't this be [^.]*, since a . means a sub-attribute?

I think . are authorized in merge-tools definitions without meaning a
sub-attribute, so we cannot exclude it from matching.

> 
> > +coreconfigitem('merge-tools', r'.*\.args$',
> > +    default="$local $base $other",
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.binary$',
> > +    default=False,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.check$',
> > +    default=list,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.checkchanged$',
> > +    default=False,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.executable$',
> > +    default=dynamicdefault,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.fixeol$',
> > +    default=False,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.gui$',
> > +    default=False,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.priority$',
> > +    default=0,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.premerge$',
> > +    default=dynamicdefault,
> > +    generic=True,
> > +    priority=-1,
> > +)
> > +coreconfigitem('merge-tools', r'.*\.symlink$',
> > +    default=False,
> > +    generic=True,
> > +    priority=-1,
> > +)
> >  coreconfigitem('pager', 'ignore',
> >      default=list,
> >  )
> > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> > --- a/mercurial/filemerge.py
> > +++ b/mercurial/filemerge.py
> > @@ -30,16 +30,14 @@
> >      util,
> >  )
> > 
> > -def _toolstr(ui, tool, part, default=""):
> > -    return ui.config("merge-tools", tool + "." + part, default)
> > +def _toolstr(ui, tool, part, *args):
> > +    return ui.config("merge-tools", tool + "." + part, *args)
> > 
> > -def _toolbool(ui, tool, part, default=False):
> > -    return ui.configbool("merge-tools", tool + "." + part,
> > default)
> > +def _toolbool(ui, tool, part,*args):
> > +    return ui.configbool("merge-tools", tool + "." + part, *args)
> > 
> > -def _toollist(ui, tool, part, default=None):
> > -    if default is None:
> > -        default = []
> > -    return ui.configlist("merge-tools", tool + "." + part,
> > default)
> > +def _toollist(ui, tool, part):
> > +    return ui.configlist("merge-tools", tool + "." + part)
> > 
> >  internals = {}
> >  # Merge tools to document.
> > @@ -187,8 +185,8 @@
> >      for k, v in ui.configitems("merge-tools"):
> >          t = k.split('.')[0]
> >          if t not in tools:
> > -            tools[t] = int(_toolstr(ui, t, "priority", "0"))
> > -        if _toolbool(ui, t, "disabled", False):
> > +            tools[t] = int(_toolstr(ui, t, "priority"))
> > +        if _toolbool(ui, t, "disabled"):
> >              disabled.add(t)
> >      names = tools.keys()
> >      tools = sorted([(-p, tool) for tool, p in tools.items()
> > @@ -328,7 +326,7 @@
> >      try:
> >          premerge = _toolbool(ui, tool, "premerge", not binary)
> >      except error.ConfigError:
> > -        premerge = _toolstr(ui, tool, "premerge").lower()
> > +        premerge = _toolstr(ui, tool, "premerge", "").lower()
> >          if premerge not in validkeep:
> >              _valid = ', '.join(["'" + v + "'" for v in validkeep])
> >              raise error.ConfigError(_("%s.premerge not valid "
> > @@ -503,7 +501,7 @@
> >                 }
> >          ui = repo.ui
> > 
> > -        args = _toolstr(ui, tool, "args", '$local $base $other')
> > +        args = _toolstr(ui, tool, "args")
> >          if "$output" in args:
> >              out, a = a, back # read input from backup, write to
> > original
> >          replace = {'local': a, 'base': b, 'other': c, 'output':
> > out}
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Oct. 17, 2017, 3:11 p.m.
On Mon, 16 Oct 2017 16:00:26 +0200, Boris Feld wrote:
> On Fri, 2017-10-13 at 15:37 -0400, Augie Fackler wrote:
> > On Fri, Oct 13, 2017 at 07:55:14PM +0200, Boris Feld wrote:
> > > # HG changeset patch
> > > # User Boris Feld <boris.feld@octobus.net>
> > > # Date 1507487833 -7200
> > > #      Sun Oct 08 20:37:13 2017 +0200
> > > # Node ID 6ad9f6f42f33fe5efb78f76d39c41f28333144d6
> > > # Parent  0824a3519c14d643a81dd4f718846f15045772ec
> > > # EXP-Topic config.register.ready
> > > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > > #              hg pull https://bitbucket.org/octobus/mercurial-deve
> > > l/ -r 6ad9f6f42f33
> > > configitems: register the full 'merge-tools' config and sub-options
> > > 
> > > We register the merge-tools config section (which has an arbitrary
> > > base config
> > > value) and the possible sub-attribute. The sub-attribute has to be
> > > registered
> > > first or at the same time otherwise the '.*' item would shadow
> > > them.
> > > 
> > > diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> > > --- a/mercurial/configitems.py
> > > +++ b/mercurial/configitems.py
> > > @@ -498,6 +498,60 @@
> > >  coreconfigitem('merge', 'preferancestor',
> > >          default=lambda: ['*'],
> > >  )
> > > +coreconfigitem('merge-tools', '.*',
> > > +    default=None,
> > > +    generic=True,
> > > +)
> > 
> > Shouldn't this be [^.]*, since a . means a sub-attribute?
> 
> I think . are authorized in merge-tools definitions without meaning a
> sub-attribute, so we cannot exclude it from matching.
> 
> > 
> > > +coreconfigitem('merge-tools', r'.*\.args$',
> > > +    default="$local $base $other",
> > > +    generic=True,
> > > +    priority=-1,
> > > +)

Maybe it's too late to change, but if the match function were re.search(),
we can easily write a suffix match without using greedy .* pattern.

Patch

diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -498,6 +498,60 @@ 
 coreconfigitem('merge', 'preferancestor',
         default=lambda: ['*'],
 )
+coreconfigitem('merge-tools', '.*',
+    default=None,
+    generic=True,
+)
+coreconfigitem('merge-tools', r'.*\.args$',
+    default="$local $base $other",
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.binary$',
+    default=False,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.check$',
+    default=list,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.checkchanged$',
+    default=False,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.executable$',
+    default=dynamicdefault,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.fixeol$',
+    default=False,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.gui$',
+    default=False,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.priority$',
+    default=0,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.premerge$',
+    default=dynamicdefault,
+    generic=True,
+    priority=-1,
+)
+coreconfigitem('merge-tools', r'.*\.symlink$',
+    default=False,
+    generic=True,
+    priority=-1,
+)
 coreconfigitem('pager', 'ignore',
     default=list,
 )
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -30,16 +30,14 @@ 
     util,
 )
 
-def _toolstr(ui, tool, part, default=""):
-    return ui.config("merge-tools", tool + "." + part, default)
+def _toolstr(ui, tool, part, *args):
+    return ui.config("merge-tools", tool + "." + part, *args)
 
-def _toolbool(ui, tool, part, default=False):
-    return ui.configbool("merge-tools", tool + "." + part, default)
+def _toolbool(ui, tool, part,*args):
+    return ui.configbool("merge-tools", tool + "." + part, *args)
 
-def _toollist(ui, tool, part, default=None):
-    if default is None:
-        default = []
-    return ui.configlist("merge-tools", tool + "." + part, default)
+def _toollist(ui, tool, part):
+    return ui.configlist("merge-tools", tool + "." + part)
 
 internals = {}
 # Merge tools to document.
@@ -187,8 +185,8 @@ 
     for k, v in ui.configitems("merge-tools"):
         t = k.split('.')[0]
         if t not in tools:
-            tools[t] = int(_toolstr(ui, t, "priority", "0"))
-        if _toolbool(ui, t, "disabled", False):
+            tools[t] = int(_toolstr(ui, t, "priority"))
+        if _toolbool(ui, t, "disabled"):
             disabled.add(t)
     names = tools.keys()
     tools = sorted([(-p, tool) for tool, p in tools.items()
@@ -328,7 +326,7 @@ 
     try:
         premerge = _toolbool(ui, tool, "premerge", not binary)
     except error.ConfigError:
-        premerge = _toolstr(ui, tool, "premerge").lower()
+        premerge = _toolstr(ui, tool, "premerge", "").lower()
         if premerge not in validkeep:
             _valid = ', '.join(["'" + v + "'" for v in validkeep])
             raise error.ConfigError(_("%s.premerge not valid "
@@ -503,7 +501,7 @@ 
                }
         ui = repo.ui
 
-        args = _toolstr(ui, tool, "args", '$local $base $other')
+        args = _toolstr(ui, tool, "args")
         if "$output" in args:
             out, a = a, back # read input from backup, write to original
         replace = {'local': a, 'base': b, 'other': c, 'output': out}