This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8845

templatespec: create a factory function for each type there is
ClosedPublic

Authored by martinvonz on Jul 29 2020, 2:38 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG8cce9f77ca73: templatespec: create a factory function for each type there is
Summary

Most of the arguments to the templatespec constructor are mutually
exclusive, so each combination creates a different type of
templatespec. Let's clarify that by creating factory functions.

I've left the callers in logcmdutil unchanged for now because they
are more complex and logcmdutil.templatespec() is slightly higher
level in that it is specific to changesets.

My larger goal is to add support frozen binaries (specifically
PyOxidizer) by adding a specific type of templatespec for built-in
templates. That will get its own factory function.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jul 29 2020, 2:38 PM
martinvonz added inline comments.Jul 29 2020, 3:23 PM
mercurial/formatter.py
582

Is unspecified_templatespec() better? Or maybe plain_templatespec()? It seems to result in a plainformatter getting used.

indygreg accepted this revision.Aug 2 2020, 12:14 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
mercurial/formatter.py
582

Naming is hard. I won't let this hold up review.

This revision is now accepted and ready to land.Aug 2 2020, 12:14 PM

Revision Contents
Changeset List

PathPackages
Mhgext/patchbomb.py (2 lines)
Mmercurial/cmdutil.py (2 lines)
Mmercurial/formatter.py (34 lines)

Diff 22186

hgext/patchbomb.py

def _formatflags(ui, repo, rev, flags): def _formatflags(ui, repo, rev, flags):
"""build flag string optionally by template""" """build flag string optionally by template"""
tmpl = ui.config(b'patchbomb', b'flagtemplate') tmpl = ui.config(b'patchbomb', b'flagtemplate')
if not tmpl: if not tmpl:
return b' '.join(flags) return b' '.join(flags)
out = util.stringio() out = util.stringio()
spec = formatter.templatespec(b'', templater.unquotestring(tmpl), None) spec = formatter.literal_templatespec(templater.unquotestring(tmpl))
with formatter.templateformatter(ui, out, b'patchbombflag', {}, spec) as fm: with formatter.templateformatter(ui, out, b'patchbombflag', {}, spec) as fm:
fm.startitem() fm.startitem()
fm.context(ctx=repo[rev]) fm.context(ctx=repo[rev])
fm.write(b'flags', b'%s', fm.formatlist(flags, name=b'flag')) fm.write(b'flags', b'%s', fm.formatlist(flags, name=b'flag'))
return out.getvalue() return out.getvalue()
def _formatprefix(ui, repo, rev, flags, idx, total, numbered): def _formatprefix(ui, repo, rev, flags, idx, total, numbered):

mercurial/cmdutil.py

if unchangedmessagedetection and editortext == templatetext: if unchangedmessagedetection and editortext == templatetext:
raise error.Abort(_(b"commit message unchanged")) raise error.Abort(_(b"commit message unchanged"))
return text return text
def buildcommittemplate(repo, ctx, subs, extramsg, ref): def buildcommittemplate(repo, ctx, subs, extramsg, ref):
ui = repo.ui ui = repo.ui
spec = formatter.templatespec(ref, None, None) spec = formatter.reference_templatespec(ref)
t = logcmdutil.changesettemplater(ui, repo, spec) t = logcmdutil.changesettemplater(ui, repo, spec)
t.t.cache.update( t.t.cache.update(
(k, templater.unquotestring(v)) (k, templater.unquotestring(v))
for k, v in repo.ui.configitems(b'committemplate') for k, v in repo.ui.configitems(b'committemplate')
) )
if not extramsg: if not extramsg:
extramsg = b'' # ensure that extramsg is string extramsg = b'' # ensure that extramsg is string

mercurial/formatter.py

@attr.s(frozen=True) @attr.s(frozen=True)
class templatespec(object): class templatespec(object):
ref = attr.ib() ref = attr.ib()
tmpl = attr.ib() tmpl = attr.ib()
mapfile = attr.ib() mapfile = attr.ib()
refargs = attr.ib(default=None) refargs = attr.ib(default=None)
def empty_templatespec():
return templatespec(None, None, None)
def reference_templatespec(ref, refargs=None):
return templatespec(ref, None, None, refargs)
def literal_templatespec(tmpl):
return templatespec(b'', tmpl, None)
def mapfile_templatespec(topic, mapfile):
return templatespec(topic, None, mapfile)
def lookuptemplate(ui, topic, tmpl): def lookuptemplate(ui, topic, tmpl):
"""Find the template matching the given -T/--template spec 'tmpl' """Find the template matching the given -T/--template spec 'tmpl'
'tmpl' can be any of the following: 'tmpl' can be any of the following:
- a literal template (e.g. '{rev}') - a literal template (e.g. '{rev}')
- a reference to built-in template (i.e. formatter) - a reference to built-in template (i.e. formatter)
- a map-file name or path (e.g. 'changelog') - a map-file name or path (e.g. 'changelog')
- a reference to [templates] in config file - a reference to [templates] in config file
- a path to raw template file - a path to raw template file
A map file defines a stand-alone template environment. If a map file A map file defines a stand-alone template environment. If a map file
selected, all templates defined in the file will be loaded, and the selected, all templates defined in the file will be loaded, and the
template matching the given topic will be rendered. Aliases won't be template matching the given topic will be rendered. Aliases won't be
loaded from user config, but from the map file. loaded from user config, but from the map file.
If no map file selected, all templates in [templates] section will be If no map file selected, all templates in [templates] section will be
available as well as aliases in [templatealias]. available as well as aliases in [templatealias].
""" """
if not tmpl: if not tmpl:
return templatespec(None, None, None) return empty_templatespec()
martinvonzAuthorUnsubmitted
Done

Is unspecified_templatespec() better? Or maybe plain_templatespec()? It seems to result in a plainformatter getting used.

martinvonz: Is `unspecified_templatespec()` better? Or maybe `plain_templatespec()`? It seems to result in…
indygregUnsubmitted
Not Done

Naming is hard. I won't let this hold up review.

indygreg: Naming is hard. I won't let this hold up review.
# looks like a literal template? # looks like a literal template?
if b'{' in tmpl: if b'{' in tmpl:
return templatespec(b'', tmpl, None) return literal_templatespec(tmpl)
# a reference to built-in (formatter) template # a reference to built-in (formatter) template
if tmpl in {b'cbor', b'json', b'pickle', b'debug'}: if tmpl in {b'cbor', b'json', b'pickle', b'debug'}:
return templatespec(tmpl, None, None) return reference_templatespec(tmpl)
# a function-style reference to built-in template # a function-style reference to built-in template
func, fsep, ftail = tmpl.partition(b'(') func, fsep, ftail = tmpl.partition(b'(')
if func in {b'cbor', b'json'} and fsep and ftail.endswith(b')'): if func in {b'cbor', b'json'} and fsep and ftail.endswith(b')'):
templater.parseexpr(tmpl) # make sure syntax errors are confined templater.parseexpr(tmpl) # make sure syntax errors are confined
return templatespec(func, None, None, refargs=ftail[:-1]) return reference_templatespec(func, refargs=ftail[:-1])
# perhaps a stock style? # perhaps a stock style?
if not os.path.split(tmpl)[0]: if not os.path.split(tmpl)[0]:
mapname = templater.templatepath( mapname = templater.templatepath(
b'map-cmdline.' + tmpl b'map-cmdline.' + tmpl
) or templater.templatepath(tmpl) ) or templater.templatepath(tmpl)
if mapname: if mapname:
return templatespec(topic, None, mapname) return mapfile_templatespec(topic, mapname)
# perhaps it's a reference to [templates] # perhaps it's a reference to [templates]
if ui.config(b'templates', tmpl): if ui.config(b'templates', tmpl):
return templatespec(tmpl, None, None) return reference_templatespec(tmpl)
if tmpl == b'list': if tmpl == b'list':
ui.write(_(b"available styles: %s\n") % templater.stylelist()) ui.write(_(b"available styles: %s\n") % templater.stylelist())
raise error.Abort(_(b"specify a template")) raise error.Abort(_(b"specify a template"))
# perhaps it's a path to a map or a template # perhaps it's a path to a map or a template
if (b'/' in tmpl or b'\\' in tmpl) and os.path.isfile(tmpl): if (b'/' in tmpl or b'\\' in tmpl) and os.path.isfile(tmpl):
# is it a mapfile for a style? # is it a mapfile for a style?
if os.path.basename(tmpl).startswith(b"map-"): if os.path.basename(tmpl).startswith(b"map-"):
return templatespec(topic, None, os.path.realpath(tmpl)) return mapfile_templatespec(topic, os.path.realpath(tmpl))
with util.posixfile(tmpl, b'rb') as f: with util.posixfile(tmpl, b'rb') as f:
tmpl = f.read() tmpl = f.read()
return templatespec(b'', tmpl, None) return literal_templatespec(tmpl)
# constant string? # constant string?
return templatespec(b'', tmpl, None) return literal_templatespec(tmpl)
def templatepartsmap(spec, t, partnames): def templatepartsmap(spec, t, partnames):
"""Create a mapping of {part: ref}""" """Create a mapping of {part: ref}"""
partsmap = {spec.ref: spec.ref} # initial ref must exist in t partsmap = {spec.ref: spec.ref} # initial ref must exist in t
if spec.mapfile: if spec.mapfile:
partsmap.update((p, p) for p in partnames if p in t) partsmap.update((p, p) for p in partnames if p in t)
elif spec.ref: elif spec.ref: