Patchwork templater: add template path to __base__ search

login
register
mail settings
Submitter Matt Mackall
Date Aug. 25, 2016, 12:48 a.m.
Message ID <1ec871261bb777e24f21.1472086108@ruin.waste.org>
Download mbox | patch
Permalink /patch/16405/
State Accepted
Headers show

Comments

Matt Mackall - Aug. 25, 2016, 12:48 a.m.
# HG changeset patch
# User Matt Mackall <mpm@selenic.com>
# Date 1472085825 25200
#      Wed Aug 24 17:43:45 2016 -0700
# Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
# Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
templater: add template path to __base__ search

This does a fall-back check for style files or directories that are
in Mercurial's template path for user convenience.

We intentionally don't use this for the built-in coal style because we don't
want the style to mysteriously break if the working directory just
happens to have a file named "paper".
Yuya Nishihara - Aug. 25, 2016, 1:42 p.m.
On Wed, 24 Aug 2016 17:48:28 -0700, Matt Mackall wrote:
> # HG changeset patch
> # User Matt Mackall <mpm@selenic.com>
> # Date 1472085825 25200
> #      Wed Aug 24 17:43:45 2016 -0700
> # Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
> # Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
> templater: add template path to __base__ search

Nice. Queued, thanks.
Pierre-Yves David - Aug. 26, 2016, 9:23 a.m.
On 08/25/2016 02:48 AM, Matt Mackall wrote:
> # HG changeset patch
> # User Matt Mackall <mpm@selenic.com>
> # Date 1472085825 25200
> #      Wed Aug 24 17:43:45 2016 -0700
> # Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
> # Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
> templater: add template path to __base__ search
>
> This does a fall-back check for style files or directories that are
> in Mercurial's template path for user convenience.

So this seems to mean we are looking for __base__ the same way we search 
for style. This seems very sensible.

> We intentionally don't use this for the built-in coal style because we don't
> want the style to mysteriously break if the working directory just
> happens to have a file named "paper".

I'm a bit more confused here. First, I would imagine our search priority 
would help two style next to each other be able to to work together. 
Second, does this means the working directory is in the template search 
path? If so how is it not an issue already?

The new feature is definitly neat and shiny, but if -we- cannot even use 
it for our most common template this looks like an hatching disaster.

Did I missed something?

(I've refrained myself to accept that changeset until this is clarified 
on my side).
Matt Mackall - Aug. 26, 2016, 9:43 a.m.
On Fri, 2016-08-26 at 11:23 +0200, Pierre-Yves David wrote:
> 
> On 08/25/2016 02:48 AM, Matt Mackall wrote:
> > 
> > # HG changeset patch
> > # User Matt Mackall <mpm@selenic.com>
> > # Date 1472085825 25200
> > #      Wed Aug 24 17:43:45 2016 -0700
> > # Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
> > # Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
> > templater: add template path to __base__ search
> > 
> > This does a fall-back check for style files or directories that are
> > in Mercurial's template path for user convenience.
> So this seems to mean we are looking for __base__ the same way we search 
> for style. This seems very sensible.
> 
> > 
> > We intentionally don't use this for the built-in coal style because we don't
> > want the style to mysteriously break if the working directory just
> > happens to have a file named "paper".
> I'm a bit more confused here. First, I would imagine our search priority 
> would help two style next to each other be able to to work together. 
> Second, does this means the working directory is in the template search 
> path? If so how is it not an issue already?

> The new feature is definitly neat and shiny, but if -we- cannot even use 
> it for our most common template this looks like an hatching disaster.

Scenario I'm slightly worried about:

- someone has a working hgweb setup with coal, has never heard of "paper"
- a file named "paper" gets added to hgweb working directory (improbable)
- coal theme mysteriously stops working due to action at a distance

Scenario I'm not worried about:

- user creates a new style that derives from paper
- user now has knowledge of how templater works and the significance of "paper"
- a file named "paper" gets added to hgweb working directory
- system breaks, but it's much less mysterious

The first scenario is unlikely, but we might as well avoid it.

-- 
Mathematics is the supreme nostalgia of our time.
Pierre-Yves David - Aug. 26, 2016, 9:51 a.m.
On 08/26/2016 11:43 AM, Matt Mackall wrote:
> On Fri, 2016-08-26 at 11:23 +0200, Pierre-Yves David wrote:
>>
>> On 08/25/2016 02:48 AM, Matt Mackall wrote:
>>>
>>> # HG changeset patch
>>> # User Matt Mackall <mpm@selenic.com>
>>> # Date 1472085825 25200
>>> #      Wed Aug 24 17:43:45 2016 -0700
>>> # Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
>>> # Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
>>> templater: add template path to __base__ search
>>>
>>> This does a fall-back check for style files or directories that are
>>> in Mercurial's template path for user convenience.
>> So this seems to mean we are looking for __base__ the same way we search
>> for style. This seems very sensible.
>>
>>>
>>> We intentionally don't use this for the built-in coal style because we don't
>>> want the style to mysteriously break if the working directory just
>>> happens to have a file named "paper".
>> I'm a bit more confused here. First, I would imagine our search priority
>> would help two style next to each other be able to to work together.
>> Second, does this means the working directory is in the template search
>> path? If so how is it not an issue already?
>
>> The new feature is definitly neat and shiny, but if -we- cannot even use
>> it for our most common template this looks like an hatching disaster.
>
> Scenario I'm slightly worried about:
>
> - someone has a working hgweb setup with coal, has never heard of "paper"
> - a file named "paper" gets added to hgweb working directory (improbable)
> - coal theme mysteriously stops working due to action at a distance
>
> Scenario I'm not worried about:
>
> - user creates a new style that derives from paper
> - user now has knowledge of how templater works and the significance of "paper"
> - a file named "paper" gets added to hgweb working directory
> - system breaks, but it's much less mysterious
>
> The first scenario is unlikely, but we might as well avoid it.

I got that, but my question is more: how do get in a situation where the 
first scenario is a loosing case in he first place?

1) Why do we have the hgweb working directory in the search path?
2) Why do the search priority makes it possible for coal to get confused?

As __base__ is a new feature it seems like we have an opportunity to get 
things right (or to explain to me why the current setup is the right one).

Cheers,
Yuya Nishihara - Aug. 26, 2016, 2:14 p.m.
On Fri, 26 Aug 2016 11:51:40 +0200, Pierre-Yves David wrote:
> On 08/26/2016 11:43 AM, Matt Mackall wrote:
> > On Fri, 2016-08-26 at 11:23 +0200, Pierre-Yves David wrote:
> >>
> >> On 08/25/2016 02:48 AM, Matt Mackall wrote:
> >>>
> >>> # HG changeset patch
> >>> # User Matt Mackall <mpm@selenic.com>
> >>> # Date 1472085825 25200
> >>> #      Wed Aug 24 17:43:45 2016 -0700
> >>> # Node ID 1ec871261bb777e24f21cb180fce199cb49c55ea
> >>> # Parent  5f86818c95e5ea59c48dfceca2e286cd11f9a800
> >>> templater: add template path to __base__ search
> >>>
> >>> This does a fall-back check for style files or directories that are
> >>> in Mercurial's template path for user convenience.
> >> So this seems to mean we are looking for __base__ the same way we search
> >> for style. This seems very sensible.
> >>
> >>>
> >>> We intentionally don't use this for the built-in coal style because we don't
> >>> want the style to mysteriously break if the working directory just
> >>> happens to have a file named "paper".
> >> I'm a bit more confused here. First, I would imagine our search priority
> >> would help two style next to each other be able to to work together.
> >> Second, does this means the working directory is in the template search
> >> path? If so how is it not an issue already?
> >
> >> The new feature is definitly neat and shiny, but if -we- cannot even use
> >> it for our most common template this looks like an hatching disaster.
> >
> > Scenario I'm slightly worried about:
> >
> > - someone has a working hgweb setup with coal, has never heard of "paper"
> > - a file named "paper" gets added to hgweb working directory (improbable)
> > - coal theme mysteriously stops working due to action at a distance
> >
> > Scenario I'm not worried about:
> >
> > - user creates a new style that derives from paper
> > - user now has knowledge of how templater works and the significance of "paper"
> > - a file named "paper" gets added to hgweb working directory
> > - system breaks, but it's much less mysterious
> >
> > The first scenario is unlikely, but we might as well avoid it.
> 
> I got that, but my question is more: how do get in a situation where the 
> first scenario is a loosing case in he first place?
> 
> 1) Why do we have the hgweb working directory in the search path?
> 2) Why do the search priority makes it possible for coal to get confused?

Good catch. I misread the working directory as a user-specified search path.
If web.templates is specified, and if the user template directory has "paper",
"coal" could be broken. But which seems the scenario mpm's not worried about.

Also, web.templates isn't honored when looking for base templates nor %include
files, which is another bug.

Patch

diff -r 5f86818c95e5 -r 1ec871261bb7 mercurial/templater.py
--- a/mercurial/templater.py	Wed Aug 17 13:43:13 2016 -0500
+++ b/mercurial/templater.py	Wed Aug 24 17:43:45 2016 -0700
@@ -1029,6 +1029,19 @@ 
         elif key == "__base__":
             # treat as a pointer to a base class for this style
             path = util.normpath(os.path.join(base, val))
+
+            # fallback check in template paths
+            if not os.path.exists(path):
+                for p in templatepaths():
+                    p2 = util.normpath(os.path.join(p, val))
+                    if os.path.isfile(p2):
+                        path = p2
+                        break
+                    p3 = util.normpath(os.path.join(p2, "map"))
+                    if os.path.isfile(p3):
+                        path = p3
+                        break
+
             bcache, btmap = _readmapfile(path)
             for k in bcache:
                 if k not in cache:
diff -r 5f86818c95e5 -r 1ec871261bb7 tests/test-command-template.t
--- a/tests/test-command-template.t	Wed Aug 17 13:43:13 2016 -0500
+++ b/tests/test-command-template.t	Wed Aug 24 17:43:45 2016 -0700
@@ -103,6 +103,18 @@ 
   $ hg log -l1 -T./map-simple
   8
 
+Test template map inheritance
+
+  $ echo "__base__ = map-cmdline.default" > map-simple
+  $ printf 'cset = "changeset: ***{rev}***\\n"\n' >> map-simple
+  $ hg log -l1 -T./map-simple
+  changeset: ***8***
+  tag:         tip
+  user:        test
+  date:        Wed Jan 01 10:01:00 2020 +0000
+  summary:     third
+  
+
 Template should precede style option
 
   $ hg log -l1 --style default -T '{rev}\n'