D1678: revset: pass pre-optimized tree in revset.matchany()

pulkit (Pulkit Goyal) phabricator at mercurial-scm.org
Thu Dec 14 18:57:18 EST 2017


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-<x>", but "<x>" 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


More information about the Mercurial-devel mailing list