Patchwork [5,of,6] config: give it a mapfile option, for parsing them specially

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 2, 2015, 4:56 a.m.
Message ID <c06e371d76b759b900b3.1430542580@Iris>
Download mbox | patch
Permalink /patch/8850/
State Rejected
Headers show

Comments

Jordi Gutiérrez Hermoso - May 2, 2015, 4:56 a.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1430344009 14400
#      Wed Apr 29 17:46:49 2015 -0400
# Node ID c06e371d76b759b900b34120d8d3e90a63790660
# Parent  5adc92b85bfe045bfd527d836eccad53a5568e92
config: give it a mapfile option, for parsing them specially

It is desirable to "derive" templates from the provided templates. A
simple way to do this is e.g.

    %include map-cmdline.default

in your own mapfile. Then you only have to redefine a few templates
instead of copying over the whole thing. This %include mechanism
already works for the built-in templates because by default it *only*
looks for files that are in the same directory as the including
mapfile.

With this changeset, config grows an option to optionally add the
templates directory to the %include path.
Yuya Nishihara - May 2, 2015, 10:49 a.m.
On Sat, 02 May 2015 00:56:20 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1430344009 14400
> #      Wed Apr 29 17:46:49 2015 -0400
> # Node ID c06e371d76b759b900b34120d8d3e90a63790660
> # Parent  5adc92b85bfe045bfd527d836eccad53a5568e92
> config: give it a mapfile option, for parsing them specially
> 
> It is desirable to "derive" templates from the provided templates. A
> simple way to do this is e.g.
> 
>     %include map-cmdline.default
> 
> in your own mapfile. Then you only have to redefine a few templates
> instead of copying over the whole thing. This %include mechanism
> already works for the built-in templates because by default it *only*
> looks for files that are in the same directory as the including
> mapfile.

I have mixed feelings about this. It must be nice that we no longer have
to write custom templates from scratch. On the other hand, this means all
template variables defined in map files are public interface of Mercurial.

> diff --git a/mercurial/config.py b/mercurial/config.py
> --- a/mercurial/config.py
> +++ b/mercurial/config.py
> @@ -8,12 +8,14 @@
>  from i18n import _
>  import error, util
>  import os, errno
> +import templater
>  
>  class config(object):
> -    def __init__(self, data=None):
> +    def __init__(self, data=None, mapfile=False):
>          self._data = {}
>          self._source = {}
>          self._unset = []
> +        self._mapfile = mapfile

Instead of making config know how templater works, how about passing
includepaths to config.config() ?

Regards,
Jordi Gutiérrez Hermoso - May 2, 2015, 3:16 p.m.
On Sat, 2015-05-02 at 19:49 +0900, Yuya Nishihara wrote:
> Instead of making config know how templater works, how about passing
> includepaths to config.config() ?

Oh, good idea. That's certainly cleaner. I'll rework the whole patch.
Pierre-Yves David - May 5, 2015, 12:45 a.m.
On 05/02/2015 03:49 AM, Yuya Nishihara wrote:
> On Sat, 02 May 2015 00:56:20 -0400, Jordi Gutiérrez Hermoso wrote:
>> # HG changeset patch
>> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
>> # Date 1430344009 14400
>> #      Wed Apr 29 17:46:49 2015 -0400
>> # Node ID c06e371d76b759b900b34120d8d3e90a63790660
>> # Parent  5adc92b85bfe045bfd527d836eccad53a5568e92
>> config: give it a mapfile option, for parsing them specially
>>
>> It is desirable to "derive" templates from the provided templates. A
>> simple way to do this is e.g.
>>
>>      %include map-cmdline.default
>>
>> in your own mapfile. Then you only have to redefine a few templates
>> instead of copying over the whole thing. This %include mechanism
>> already works for the built-in templates because by default it *only*
>> looks for files that are in the same directory as the including
>> mapfile.
>
> I have mixed feelings about this. It must be nice that we no longer have
> to write custom templates from scratch. On the other hand, this means all
> template variables defined in map files are public interface of Mercurial.

Template is an awesome feature of Mercurial and that seems a good way to 
push it forward. could we have a public/_private notation to "reduce" 
this effect?

Patch

diff --git a/mercurial/config.py b/mercurial/config.py
--- a/mercurial/config.py
+++ b/mercurial/config.py
@@ -8,12 +8,14 @@ 
 from i18n import _
 import error, util
 import os, errno
+import templater
 
 class config(object):
-    def __init__(self, data=None):
+    def __init__(self, data=None, mapfile=False):
         self._data = {}
         self._source = {}
         self._unset = []
+        self._mapfile = mapfile
         if data:
             for k in data._data:
                 self._data[k] = data[k].copy()
@@ -110,18 +112,29 @@  class config(object):
                 item = None
                 cont = False
             m = includere.match(l)
-            if m:
-                inc = util.expandpath(m.group(1))
-                base = os.path.dirname(src)
-                inc = os.path.normpath(os.path.join(base, inc))
+            def includefile(inc):
                 if include:
                     try:
                         include(inc, remap=remap, sections=sections)
+                        return True
                     except IOError, inst:
                         if inst.errno != errno.ENOENT:
                             raise error.ParseError(_("cannot include %s (%s)")
                                                    % (inc, inst.strerror),
                                                    "%s:%s" % (src, line))
+                        else:
+                            return False
+            if m:
+                expanded = util.expandpath(m.group(1))
+                base = os.path.dirname(src)
+                inc = os.path.normpath(os.path.join(base, expanded))
+
+                if not includefile(inc) and self._mapfile:
+                    # For template mapfiles, try to include from the
+                    # template directory, after looking in the current
+                    # directory already failed.
+                    includefile(templater.templatepath(expanded))
+
                 continue
             if emptyre.match(l):
                 continue