D1678: revset: pass pre-optimized tree in revset.matchany()
pulkit abandoned this revision. pulkit added a comment. In https://phab.mercurial-scm.org/D1678#29063, @yuja wrote: > > Since we are calling them at each command level, it will be better to call unhidehashlikerevs() in `scmutil.revrange|revsingle`. > > I thought about that, but `scmutil.rev*()` would have to return new filtered `repo` > object, which didn't seem nice. And mutating a given `repo` argument has the > original problem, when to discard unhidden revisions. So my conclusion was a > separate function. > > > We will still need to store the filtername in dispatch and then later on > > decide on basis of that whether we want to uhide things. > > Doesn't each command know whether it should unhide or not? Thanks for all the ideas, I have send the new series as https://phab.mercurial-scm.org/D1730 - https://phab.mercurial-scm.org/D1735 as they are entirely different changesets. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1678 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1678: revset: pass pre-optimized tree in revset.matchany()
yuja added a comment. > Since we are calling them at each command level, it will be better to call unhidehashlikerevs() in `scmutil.revrange|revsingle`. I thought about that, but `scmutil.rev*()` would have to return new filtered `repo` object, which didn't seem nice. And mutating a given `repo` argument has the original problem, when to discard unhidden revisions. So my conclusion was a separate function. > We will still need to store the filtername in dispatch and then later on > decide on basis of that whether we want to uhide things. Doesn't each command know whether it should unhide or not? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1678 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1678: revset: pass pre-optimized tree in revset.matchany()
pulkit added a comment.
In https://phab.mercurial-scm.org/D1678#28729, @yuja wrote:
> In https://phab.mercurial-scm.org/D1678#28686, @pulkit wrote:
>
> > @yuja am I headed in the right direction? (I am not sure about whether
the API's I changed are used by extensions or not)
>
>
> Sort of, but I was thinking of a simpler one. Just parse specs twice, which
isn't
> costly operation.
>
> def unhidehashlikerevs(repo, specs, allowhidden|warnhidden):
> if not repo.filtername:
> return repo
> syms = set()
> for spec in specs:
> try:
> tree = revsetlang.parse(spec)
> except ParseError:
> continue # will be reported later by scmutil.revrange()
> syms.update(revsetlang.hashlikesymbols(tree))
> if not syms:
> return repo
> pinned = perhaps_need_to_convert_to_nodes_or_revs?(syms)
> # filtered repo with the given extra pinned revisions; no idea if
this is an ideal API
> return repo.filtered(repo.filtername, pinned)
>
>
> This allows us to get rid of an intermediate state where a repo is flagged
as
> "visible-", but "" isn't added yet.
>
> We can apply this function at dispatch:
>
> def _dispatch(req):
> ...
> if repo:
> ui = repo.ui
> if options['hidden']:
> repo = repo.unfiltered()
> else:
> # perhaps we'll need a registrar flag to get
arguments taking revspecs
> repo = scmutil.unhidehashlikerevs(repo,
cmdoptions.get('rev', []))
>
>
> or simply at each command:
>
> def diff(...):
> ...
> repo = scmutil.unhidehashlikerevs(repo, changesets, 'allowhidden')
> revs = scmutil.revrange(repo, changesets)
>
I like your idea. Thanks for it. After some hacking, I think we should call
`scmutil.unhidehashlikerevs()` from each command level because at dispatch
level we are not sure what `revs` are, some commands don't need `--rev` to take
a rev like `hg export`. Since we are calling them at each command level, it
will be better to call unhidehashlikerevs() in `scmutil.revrange|revsingle`. We
will still need to store the filtername in dispatch and then later on decide on
basis of that whether we want to uhide things.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1678
To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1678: revset: pass pre-optimized tree in revset.matchany()
yuja added a comment.
In https://phab.mercurial-scm.org/D1678#28686, @pulkit wrote:
> @yuja am I headed in the right direction? (I am not sure about whether the
API's I changed are used by extensions or not)
Sort of, but I was thinking of a simpler one. Just parse specs twice, which
isn't
costly operation.
def unhidehashlikerevs(repo, specs, allowhidden|warnhidden):
if not repo.filtername:
return repo
syms = set()
for spec in specs:
try:
tree = revsetlang.parse(spec)
except ParseError:
continue # will be reported later by scmutil.revrange()
syms.update(revsetlang.hashlikesymbols(tree))
if not syms:
return repo
pinned = perhaps_need_to_convert_to_nodes_or_revs?(syms)
# filtered repo with the given extra pinned revisions; no idea if this
is an ideal API
return repo.filtered(repo.filtername, pinned)
This allows us to get rid of an intermediate state where a repo is flagged as
"visible-", but "" isn't added yet.
We can apply this function at dispatch:
def _dispatch(req):
...
if repo:
ui = repo.ui
if options['hidden']:
repo = repo.unfiltered()
else:
# perhaps we'll need a registrar flag to get arguments
taking revspecs
repo = scmutil.unhidehashlikerevs(repo,
cmdoptions.get('rev', []))
or simply at each command:
def diff(...):
...
repo = scmutil.unhidehashlikerevs(repo, changesets, 'allowhidden')
revs = scmutil.revrange(repo, changesets)
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1678
To: pulkit, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1678: revset: pass pre-optimized tree in revset.matchany()
pulkit added a subscriber: yuja. pulkit added a comment. @yuja am I headed in the right direction? (I am not sure about whether the API's I changed are used by extensions or not) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1678 To: pulkit, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1678: revset: pass pre-optimized tree in revset.matchany()
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
This is a part of series where we move logic around so that we can get the
tree
at a higher level function such as repo.anyrevs(). This will help us in
reading
the tree and doing certain operations like updating visibility exceptions on
base of that.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D1678
AFFECTED FILES
mercurial/localrepo.py
mercurial/revset.py
CHANGE DETAILS
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2163,26 +2163,25 @@
def match(ui, spec, repo=None):
"""Create a matcher for a single revision spec"""
-return matchany(ui, [spec], repo=repo)
+
+if not specs:
+return emptymatcher
+if not all(specs):
+raise error.ParseError(_("empty query"))
+
+tree = buildtree([spec], repo)
+return matchany(ui, tree, repo=repo)
def emptymatcher(repo, subset=None):
""" Matcher for empty specs """
return baseset()
-def matchany(ui, specs, repo=None, localalias=None):
-"""Create a matcher that will include any revisions matching one of the
-given specs
+def matchany(ui, tree, repo=None, localalias=None):
+"""Create a matcher for the tree
If localalias is not None, it is a dict {name: definitionstring}. It takes
precedence over [revsetalias] config section.
"""
-if not specs:
-return emptymatcher
-if not all(specs):
-raise error.ParseError(_("empty query"))
-
-tree = buildtree(specs, repo)
-
aliases = []
warn = None
if ui:
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -804,11 +804,19 @@
definitions overriding user aliases, set ``localalias`` to
``{name: definitionstring}``.
'''
+
+if not specs:
+return revset.emptymatcher(self)
+if not all(specs):
+raise error.ParseError(_("empty query"))
+
if user:
-m = revset.matchany(self.ui, specs, repo=self,
+tree = revset.buildtree(specs, self)
+m = revset.matchany(self.ui, tree, repo=self,
localalias=localalias)
else:
-m = revset.matchany(None, specs, localalias=localalias)
+tree = revset.buildtree(specs)
+m = revset.matchany(None, tree, localalias=localalias)
return m(self)
def url(self):
To: pulkit, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
