This is an archive of the discontinued Mercurial Phabricator instance.

Differential D97

revset: pass repo when passing ui
ClosedPublic

Authored by indygreg on Jul 15 2017, 7:41 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG2943141f5e07: revset: pass repo when passing ui
rHG2858f7d10344: revset: pass repo when passing ui
Summary

The repo instance is currently only used to provide a changeset
lookup function as part of parsing revsets. I /think/ this allows
node fragments to resolve. I'm not sure why we wouldn't want this
to always "just work" if parsing a revset string.

Plus, an upcoming commit will introduce a new consumer that needs a
handle on the repo. So passing it more often will make that code
work more.

Passing a repo instance in all callers of revset.match* results in
a bunch of test changes. Notably, branch and tags caches get
populated as part of evaluating revsets. I'm not sure if this is
desirable. So this patch takes the conservative approach and only
passes the repo if we're passing a ui instance.

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

indygreg created this revision.Jul 15 2017, 7:41 PM
durin42 accepted this revision.Jul 17 2017, 4:22 PM
durin42 added a subscriber: durin42.

Taking this one, but going to punt on D98 per the "consider this an RFC" comment I saw.

This revision is now accepted and ready to land.Jul 17 2017, 4:22 PM
yuja added a subscriber: yuja.Jul 18 2017, 8:51 AM
yuja added inline comments.
mercurial/cmdutil.py
2479

Perhaps these two shouldn't pass ui nor repo. They evaluate
internal query.

Passing repo means to take any valid old-style revisions
ignoring the revset syntax.

Closed by commit rHG2858f7d10344: revset: pass repo when passing ui (authored by indygreg). · Explain WhyJul 18 2017, 2:41 PM
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.Jul 18 2017, 4:30 PM
martinvonz added inline comments.
mercurial/cmdutil.py
2479

Thanks, Yuja. This was fixed in flight by someone.

Revision Contents
Changeset List

Diff 275

mercurial/cmdutil.py

return smartset.baseset(), None, None return smartset.baseset(), None, None
expr, filematcher = _makelogrevset(repo, pats, opts, revs) expr, filematcher = _makelogrevset(repo, pats, opts, revs)
if opts.get('rev'): if opts.get('rev'):
# User-specified revs might be unsorted, but don't sort before # User-specified revs might be unsorted, but don't sort before
# _makelogrevset because it might depend on the order of revs # _makelogrevset because it might depend on the order of revs
if not (revs.isdescending() or revs.istopo()): if not (revs.isdescending() or revs.istopo()):
revs.sort(reverse=True) revs.sort(reverse=True)
if expr: if expr:
matcher = revset.match(repo.ui, expr, order=revset.followorder) matcher = revset.match(repo.ui, expr, repo=repo,
order=revset.followorder)
revs = matcher(repo, revs) revs = matcher(repo, revs)
if limit is not None: if limit is not None:
limitedrevs = [] limitedrevs = []
for idx, rev in enumerate(revs): for idx, rev in enumerate(revs):
if idx >= limit: if idx >= limit:
break break
limitedrevs.append(rev) limitedrevs.append(rev)
revs = smartset.baseset(limitedrevs) revs = smartset.baseset(limitedrevs)
filtering the files to be detailed when displaying the revision. filtering the files to be detailed when displaying the revision.
""" """
limit = loglimit(opts) limit = loglimit(opts)
revs = _logrevs(repo, opts) revs = _logrevs(repo, opts)
if not revs: if not revs:
return smartset.baseset([]), None, None return smartset.baseset([]), None, None
expr, filematcher = _makelogrevset(repo, pats, opts, revs) expr, filematcher = _makelogrevset(repo, pats, opts, revs)
if expr: if expr:
matcher = revset.match(repo.ui, expr, order=revset.followorder) matcher = revset.match(repo.ui, expr, repo=repo,
order=revset.followorder)
yujaUnsubmitted
Not Done

Perhaps these two shouldn't pass ui nor repo. They evaluate
internal query.

Passing repo means to take any valid old-style revisions
ignoring the revset syntax.

yuja: Perhaps these two shouldn't pass `ui` nor `repo`. They evaluate internal query. Passing `repo`…
martinvonzUnsubmitted
Not Done

Thanks, Yuja. This was fixed in flight by someone.

martinvonz: Thanks, Yuja. This was fixed in flight by someone.
revs = matcher(repo, revs) revs = matcher(repo, revs)
if limit is not None: if limit is not None:
limitedrevs = [] limitedrevs = []
for idx, r in enumerate(revs): for idx, r in enumerate(revs):
if limit <= idx: if limit <= idx:
break break
limitedrevs.append(r) limitedrevs.append(r)
revs = smartset.baseset(limitedrevs) revs = smartset.baseset(limitedrevs)

mercurial/hgweb/webcommands.py

if any((token, (value or '')[:3]) == ('string', 're:') if any((token, (value or '')[:3]) == ('string', 're:')
for token, value, pos in revsetlang.tokenize(revdef)): for token, value, pos in revsetlang.tokenize(revdef)):
return MODE_KEYWORD, query return MODE_KEYWORD, query
funcsused = revsetlang.funcsused(tree) funcsused = revsetlang.funcsused(tree)
if not funcsused.issubset(revset.safesymbols): if not funcsused.issubset(revset.safesymbols):
return MODE_KEYWORD, query return MODE_KEYWORD, query
mfunc = revset.match(web.repo.ui, revdef) mfunc = revset.match(web.repo.ui, revdef, repo=web.repo)
try: try:
revs = mfunc(web.repo) revs = mfunc(web.repo)
return MODE_REVSET, revs return MODE_REVSET, revs
# ParseError: wrongly placed tokens, wrongs arguments, etc # ParseError: wrongly placed tokens, wrongs arguments, etc
# RepoLookupError: no such revision, e.g. in 'revision:' # RepoLookupError: no such revision, e.g. in 'revision:'
# Abort: bookmark/tag not exists # Abort: bookmark/tag not exists
# LookupError: ambiguous identifier, e.g. in '(bc)' on a large repo # LookupError: ambiguous identifier, e.g. in '(bc)' on a large repo
except (error.ParseError, error.RepoLookupError, error.Abort, except (error.ParseError, error.RepoLookupError, error.Abort,

mercurial/templater.py

# i18n: "revset" is a keyword # i18n: "revset" is a keyword
raise error.ParseError(_("revset expects one or more arguments")) raise error.ParseError(_("revset expects one or more arguments"))
raw = evalstring(context, mapping, args[0]) raw = evalstring(context, mapping, args[0])
ctx = mapping['ctx'] ctx = mapping['ctx']
repo = ctx.repo() repo = ctx.repo()
def query(expr): def query(expr):
m = revsetmod.match(repo.ui, expr) m = revsetmod.match(repo.ui, expr, repo=repo)
return m(repo) return m(repo)
if len(args) > 1: if len(args) > 1:
formatargs = [evalfuncarg(context, mapping, a) for a in args[1:]] formatargs = [evalfuncarg(context, mapping, a) for a in args[1:]]
revs = query(revsetlang.formatspec(raw, *formatargs)) revs = query(revsetlang.formatspec(raw, *formatargs))
revs = list(revs) revs = list(revs)
else: else:
revsetcache = mapping['cache'].setdefault("revsetcache", {}) revsetcache = mapping['cache'].setdefault("revsetcache", {})