Patchwork [3,of,6] formatter: load templates section like a map file

# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1492859695 -32400
#      Sat Apr 22 20:14:55 2017 +0900
# Node ID 32ea66830e68419c6c97c0bcfc53f053f628b399
# Parent  06b1ac59547e8ef086bae9bc51740318c7661e7e
formatter: load templates section like a map file

Since a map file has another level to select a template (spec -> mapfile
-> topic), this isn't exactly the same as how a map file works. But I believe
most users would expect the new behavior.

A literal template is stored as an unnamed template so that it will never
conflict with the templates defined in [templates] section.

On Fri, Jun 16, 2017 at 9:37 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1492859695 -32400
> #      Sat Apr 22 20:14:55 2017 +0900
> # Node ID 32ea66830e68419c6c97c0bcfc53f053f628b399
> # Parent  06b1ac59547e8ef086bae9bc51740318c7661e7e
> formatter: load templates section like a map file
>
> Since a map file has another level to select a template (spec -> mapfile
> -> topic), this isn't exactly the same as how a map file works. But I believe
> most users would expect the new behavior.
>
> A literal template is stored as an unnamed template so that it will never
> conflict with the templates defined in [templates] section.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1649,7 +1649,10 @@ class changeset_templater(changeset_prin
>                      self.t(self._parts['footer'], **props))
>
>  def logtemplatespec(tmpl, mapfile):
> -    return formatter.templatespec('changeset', tmpl, mapfile)
> +    if mapfile:
> +        return formatter.templatespec('changeset', tmpl, mapfile)
> +    else:
> +        return formatter.templatespec('', tmpl, None)
>
>  def _lookuplogtemplate(ui, tmpl, style):
>      """Find the template matching the given template spec or style
> @@ -1706,7 +1709,7 @@ def show_changeset(ui, repo, opts, buffe
>
>      spec = _lookuplogtemplate(ui, opts.get('template'), opts.get('style'))
>
> -    if not spec.tmpl and not spec.mapfile:
> +    if not spec.ref and not spec.tmpl and not spec.mapfile:
>          return changeset_printer(ui, repo, matchfn, opts, buffered)
>
>      return changeset_templater(ui, repo, spec, matchfn, opts, buffered)
> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
> --- a/mercurial/formatter.py
> +++ b/mercurial/formatter.py
> @@ -391,11 +391,14 @@ def lookuptemplate(ui, topic, tmpl):
>      selected, all templates defined in the file will be loaded, and the
>      template matching the given topic will be rendered. No aliases will be
>      loaded from user config.
> +
> +    If no map file selected, all templates in [templates] section will be
> +    available as well as aliases in [templatealias].
>      """
>
>      # looks like a literal template?
>      if '{' in tmpl:
> -        return templatespec(topic, tmpl, None)
> +        return templatespec('', tmpl, None)
>
>      # perhaps a stock style?
>      if not os.path.split(tmpl)[0]:
> @@ -405,9 +408,8 @@ def lookuptemplate(ui, topic, tmpl):
>              return templatespec(topic, None, mapname)
>
>      # perhaps it's a reference to [templates]
> -    t = ui.config('templates', tmpl)
> -    if t:
> -        return templatespec(topic, templater.unquotestring(t), None)
> +    if ui.config('templates', tmpl):
> +        return templatespec(tmpl, None, None)
>
>      if tmpl == 'list':
>          ui.write(_("available styles: %s\n") % templater.stylelist())
> @@ -420,10 +422,10 @@ def lookuptemplate(ui, topic, tmpl):
>              return templatespec(topic, None, os.path.realpath(tmpl))
>          with util.posixfile(tmpl, 'rb') as f:
>              tmpl = f.read()
> -        return templatespec(topic, tmpl, None)
> +        return templatespec('', tmpl, None)
>
>      # constant string?
> -    return templatespec(topic, tmpl, None)
> +    return templatespec('', tmpl, None)
>
>  def loadtemplater(ui, spec, cache=None):
>      """Create a templater from either a literal template or loading from
> @@ -440,6 +442,8 @@ def maketemplater(ui, tmpl, cache=None):
>  def _maketemplater(ui, topic, tmpl, cache=None):
>      aliases = ui.configitems('templatealias')
>      t = templater.templater(cache=cache, aliases=aliases)
> +    t.cache.update((k, templater.unquotestring(v))
> +                   for k, v in ui.configitems('templates'))
>      if tmpl:
>          t.cache[topic] = tmpl
>      return t
> diff --git a/mercurial/help/templates.txt b/mercurial/help/templates.txt
> --- a/mercurial/help/templates.txt
> +++ b/mercurial/help/templates.txt
> @@ -109,6 +109,14 @@ defines a template, ``nodedate``, which
>
>    $ hg log -r . -Tnodedate
>
> +A template defined in ``templates`` section can also be referenced from
> +another template::
> +
> +  $ hg log -r . -T "{rev} {nodedate}"
> +
> +but be aware that the keywords cannot be overridden by templates. For example,
> +a template defined as ``templates.rev`` cannot be referenced as ``{rev}``.
> +
>  Examples
>  ========
>
> diff --git a/tests/test-command-template.t b/tests/test-command-template.t
> --- a/tests/test-command-template.t
> +++ b/tests/test-command-template.t
> @@ -209,14 +209,29 @@ Make sure user/global hgrc does not affe
>
>  Add some simple styles to settings
>
> -  $ echo '[templates]' >> .hg/hgrc
> -  $ printf 'simple = "{rev}\\n"\n' >> .hg/hgrc
> -  $ printf 'simple2 = {rev}\\n\n' >> .hg/hgrc
> +  $ cat <<'EOF' >> .hg/hgrc
> +  > [templates]
> +  > simple = "{rev}\n"
> +  > simple2 = {rev}\n
> +  > rev = "should not precede {rev} keyword\n"

Why would you want to do this? In other words, why don't we
reject/warn/ignore that instead? I'm guessing it's because we already
have supported this form, so we don't want to break BC?

> +  > EOF
>
>    $ hg log -l1 -Tsimple
>    8
>    $ hg log -l1 -Tsimple2
>    8
> +  $ hg log -l1 -Trev
> +  should not precede 8 keyword
> +  $ hg log -l1 -T '{simple}'
> +  8
> +
> +Map file shouldn't see user templates:
> +
> +  $ cat <<EOF > tmpl
> +  > changeset = 'nothing expanded:{simple}\n'
> +  > EOF
> +  $ hg log -l1 --style ./tmpl
> +  nothing expanded:
>
>  Test templates and style maps in files:
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Fri, 16 Jun 2017 10:59:28 -0700, Martin von Zweigbergk wrote:
> On Fri, Jun 16, 2017 at 9:37 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1492859695 -32400
> > #      Sat Apr 22 20:14:55 2017 +0900
> > # Node ID 32ea66830e68419c6c97c0bcfc53f053f628b399
> > # Parent  06b1ac59547e8ef086bae9bc51740318c7661e7e
> > formatter: load templates section like a map file

> > -  $ echo '[templates]' >> .hg/hgrc
> > -  $ printf 'simple = "{rev}\\n"\n' >> .hg/hgrc
> > -  $ printf 'simple2 = {rev}\\n\n' >> .hg/hgrc
> > +  $ cat <<'EOF' >> .hg/hgrc
> > +  > [templates]
> > +  > simple = "{rev}\n"
> > +  > simple2 = {rev}\n
> > +  > rev = "should not precede {rev} keyword\n"
> 
> Why would you want to do this? In other words, why don't we
> reject/warn/ignore that instead? I'm guessing it's because we already
> have supported this form, so we don't want to break BC?

templates.rev, the definition, is still valid since it can be referenced
by '-T rev'. As the built-in keywords space may grow, we can't reliably
reject template definitions conflicting with the future builtins.

Another reason is that hgweb templates rely on this weird behavior.