| Submitter | Jun Wu |
|---|---|
| Date | June 24, 2017, 5:04 a.m. |
| Message ID | <a8a42cc07de6538e92ba.1498280680@x1c> |
| Download | mbox | patch |
| Permalink | /patch/21658/ |
| State | Changes Requested |
| Headers | show |
Comments
On Fri, 23 Jun 2017 22:04:40 -0700, Jun Wu wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1498279695 25200 > # Fri Jun 23 21:48:15 2017 -0700 > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98 > # Parent 66117dae87f98789a217e90deb86908927967630 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6 > revset: make repo.anyrevs accept customized alias override > > Previously repo.anyrevs only expand aliases in [revsetalias] config. This > patch makes it more flexible to accept a customized dict defining aliases > without having to couple with ui. > > revsetlang.expandaliases now have the signature (tree, aliases, warn=None) > which is more consistent with templater.expandaliases. revsetlang.py is now > free from "ui", which seems to be a good thing. Yeah, sounds good. One nit. > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py > --- a/mercurial/localrepo.py > +++ b/mercurial/localrepo.py > @@ -622,14 +622,15 @@ class localrepository(object): > yield self[r] > > - def anyrevs(self, specs, user=False): > + def anyrevs(self, specs, user=False, local=None): > '''Find revisions matching one of the given revsets. > > Revset aliases from the configuration are not expanded by default. To > - expand user aliases, specify ``user=True``. > + expand user aliases, specify ``user=True``. To provide some local > + definitions, set local to {name: definition}. > ''' > if user: > - m = revset.matchany(self.ui, specs, repo=self) > + m = revset.matchany(self.ui, specs, repo=self, local=local) > else: > - m = revset.matchany(None, specs) > + m = revset.matchany(None, specs, local=local) Can you rename 'local' to 'localaliases' or something in V2? It seems too ambiguous. 'user' might not be a good name, but at least it means processing "user revset." If V2 isn't planned, I can rename it in flight. (I don't follow the IRC discussion.)
Excerpts from Yuya Nishihara's message of 2017-06-24 17:40:14 +0900: > Can you rename 'local' to 'localaliases' or something in V2? It seems too > ambiguous. 'user' might not be a good name, but at least it means > processing "user revset." Sure. The name was "inspired" by "code.interact(local=objects)". I didn't realize it's not clear whether "local" is bool or not given "user" is bool. > If V2 isn't planned, I can rename it in flight. (I don't follow the IRC > discussion.) This series was not written lightly, like "defineparents" has been rewritten 3 times taking me 2 full days to reach the elegance level that satisfies me. Therefore, although it's a non-trivial change, it's less like RFC but more of a thoughtful refactoring. I did wish it to be accepted first time. But since there are 2 comments already and I have discovered another small nit, I'll send V2 next Tuesday.
On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1498279695 25200 > # Fri Jun 23 21:48:15 2017 -0700 > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98 > # Parent 66117dae87f98789a217e90deb86908927967630 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6 > revset: make repo.anyrevs accept customized alias override I still plan to review this series. The last few days have just been very busy. I still have many more hours of other work that needs to get done first, but I'll try to get to this as soon as I can.
Excerpts from Martin von Zweigbergk's message of 2017-06-27 16:33:52 -0700: > On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote: > > # HG changeset patch > > # User Jun Wu <quark@fb.com> > > # Date 1498279695 25200 > > # Fri Jun 23 21:48:15 2017 -0700 > > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98 > > # Parent 66117dae87f98789a217e90deb86908927967630 > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6 > > revset: make repo.anyrevs accept customized alias override > > I still plan to review this series. The last few days have just been > very busy. I still have many more hours of other work that needs to > get done first, but I'll try to get to this as soon as I can. Thanks! The series could be simplified a bit after "scmutil.cleanupnodes" and "drawdag" chagnes. Therefore it might be easier to get some conclusion on those changes first (which I think are relatively strightfoward).
On Tue, Jun 27, 2017 at 5:07 PM, Jun Wu <quark@fb.com> wrote: > Excerpts from Martin von Zweigbergk's message of 2017-06-27 16:33:52 -0700: >> On Fri, Jun 23, 2017 at 10:04 PM, Jun Wu <quark@fb.com> wrote: >> > # HG changeset patch >> > # User Jun Wu <quark@fb.com> >> > # Date 1498279695 25200 >> > # Fri Jun 23 21:48:15 2017 -0700 >> > # Node ID a8a42cc07de6538e92bae498882144f9b8296c98 >> > # Parent 66117dae87f98789a217e90deb86908927967630 >> > # Available At https://bitbucket.org/quark-zju/hg-draft >> > # hg pull https://bitbucket.org/quark-zju/hg-draft -r a8a42cc07de6 >> > revset: make repo.anyrevs accept customized alias override >> >> I still plan to review this series. The last few days have just been >> very busy. I still have many more hours of other work that needs to >> get done first, but I'll try to get to this as soon as I can. > > Thanks! The series could be simplified a bit after "scmutil.cleanupnodes" > and "drawdag" chagnes. Therefore it might be easier to get some conclusion > on those changes first (which I think are relatively strightfoward). The cleanupnodes itself is already in. Do you want the patch to make rebase use it in before this series too? Also, what are the drawdag changes?
Excerpts from Martin von Zweigbergk's message of 2017-06-28 10:20:42 -0700: > The cleanupnodes itself is already in. Do you want the patch to make > rebase use it in before this series too? I was referring to use "cleanupnodes" in rebase: rebase: use scmutil.cleanupnodes (issue5606) (BC) 2017-06-26 scmutil: make cleanupnodes delete divergent bookmarks 2017-06-26 scmutil: make cleanupnodes handle filtered node 2017-06-26 That will remove bookmark movement code for example which is also changed by the multi-dest series. > Also, what are the drawdag changes? Seems I forgot to send them or didn't check the error code of "hg email". They will be: drawdag: support obsmarker creation in comments 2017-06-27 test-drawdag: add a test for drawdag.py 2017-06-26 That makes it easier to create repos with obsmarkers.
Patch
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py --- a/mercurial/debugcommands.py +++ b/mercurial/debugcommands.py @@ -1922,7 +1922,9 @@ def debugrevspec(ui, repo, expr, **opts) one. Returns 1 if the optimized result differs. """ + aliases = ui.configitems('revsetalias') stages = [ ('parsed', lambda tree: tree), - ('expanded', lambda tree: revsetlang.expandaliases(ui, tree)), + ('expanded', lambda tree: revsetlang.expandaliases(tree, aliases, + ui.warn)), ('concatenated', revsetlang.foldconcat), ('analyzed', revsetlang.analyze), diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -622,14 +622,15 @@ class localrepository(object): yield self[r] - def anyrevs(self, specs, user=False): + def anyrevs(self, specs, user=False, local=None): '''Find revisions matching one of the given revsets. Revset aliases from the configuration are not expanded by default. To - expand user aliases, specify ``user=True``. + expand user aliases, specify ``user=True``. To provide some local + definitions, set local to {name: definition}. ''' if user: - m = revset.matchany(self.ui, specs, repo=self) + m = revset.matchany(self.ui, specs, repo=self, local=local) else: - m = revset.matchany(None, specs) + m = revset.matchany(None, specs, local=local) return m(self) diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -1976,5 +1976,5 @@ def match(ui, spec, repo=None, order=def return matchany(ui, [spec], repo=repo, order=order) -def matchany(ui, specs, repo=None, order=defineorder): +def matchany(ui, specs, repo=None, order=defineorder, local=None): """Create a matcher that will include any revisions matching one of the given specs @@ -1982,4 +1982,7 @@ def matchany(ui, specs, repo=None, order If order=followorder, a matcher takes the ordering specified by the input set. + + If local is not None, it is an dict {name: definitionstring} which will + be used. It takes precedence over [revsetalias] config section. """ if not specs: @@ -1998,6 +2001,13 @@ def matchany(ui, specs, repo=None, order ('list',) + tuple(revsetlang.parse(s, lookup) for s in specs)) + aliases = [] + warn = None if ui: - tree = revsetlang.expandaliases(ui, tree) + aliases.extend(ui.configitems('revsetalias')) + warn = ui.warn + if local: + aliases.extend(local.items()) + if aliases: + tree = revsetlang.expandaliases(tree, aliases, warn=warn) tree = revsetlang.foldconcat(tree) tree = revsetlang.analyze(tree, order) diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py --- a/mercurial/revsetlang.py +++ b/mercurial/revsetlang.py @@ -562,12 +562,14 @@ class _aliasrules(parser.basealiasrules) return tree[1][1], getlist(tree[2]) -def expandaliases(ui, tree): - aliases = _aliasrules.buildmap(ui.configitems('revsetalias')) +def expandaliases(tree, aliases, warn=None): + """Expand aliases in a tree, aliases is a list of (name, value) tuples""" + aliases = _aliasrules.buildmap(aliases) tree = _aliasrules.expand(aliases, tree) # warn about problematic (but not referred) aliases - for name, alias in sorted(aliases.iteritems()): - if alias.error and not alias.warned: - ui.warn(_('warning: %s\n') % (alias.error)) - alias.warned = True + if warn is not None: + for name, alias in sorted(aliases.iteritems()): + if alias.error and not alias.warned: + warn(_('warning: %s\n') % (alias.error)) + alias.warned = True return tree diff --git a/tests/test-revset.t b/tests/test-revset.t --- a/tests/test-revset.t +++ b/tests/test-revset.t @@ -4070,3 +4070,25 @@ loading it [255] +Test repo.anyrevs with customized revset overrides + + $ cat > $TESTTMP/printprevset.py <<EOF + > from mercurial import encoding + > def reposetup(ui, repo): + > local = {} + > p = encoding.environ.get('P') + > if p: + > local['P'] = p + > ui.write('P=%r' % list(repo.anyrevs(['P'], user=True, local=local))) + > EOF + + $ cat >> .hg/hgrc <<EOF + > custompredicate = ! + > printprevset = $TESTTMP/printprevset.py + > EOF + + $ hg --config revsetalias.P=1 log -r . -T '\n' + P=[1] + $ P=3 hg --config revsetalias.P=2 log -r . -T '\n' + P=[3] + $ cd ..