D3024: scmutil: add method for looking up a context given a revision symbol

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Tue Apr 3 11:31:30 EDT 2018


martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3024#48946, @yuja wrote:
  
  > Seems fine.
  >
  > What the final state of `repo[x]`, `x in repo`, and `repo.lookup(x)` will be?
  
  
  `repo.lookup(x)` will simply delegate to `scmutil.revsymbol(x)`. IIUC, `repo.lookup()` is there partly to serve the "lookup" wire protocol command, so it makes sense to me to have it call `scmutil.revsymbol()`. Btw, perhaps the reason one can't do `repo['.^']` is because `repo[x]` was backing the wire protocol command, and we didn't want to expose that functionality. If we remove support for looking up branches etc from `repo[x]`, then perhaps we can add it back to a new method, if we think that's useful enough. Since that new method won't be backing the wire protocol command, I think we might as well make it support revsets so `repo['.^']` would work. Note that there's already a `repo.changectx()` that seems to have very few users and perhaps we could repurpose it.
  
  `repo[x]` and `x in repo` won't change (but `changectx.__init__` will be much smaller), at least not to start with. Once `changectx.__init__` only accepts a nodeid or a revnum, then perhaps we should move that into `repo[x]`, but that's far down the road. I'm a little hesitant to drop support for `repo['.']` and tell people to use `repo.lookup('.')`, even though that seems like the right thing to do.
  
  In hindsight, I'm actually a little surprised we didn't have a function like revsymbol() before. It seems like an obvious thing to have, but I guess for historical reasons it ended up being inside changectx's constructor.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3024

To: martinvonz, #hg-reviewers, yuja
Cc: mercurial-devel


More information about the Mercurial-devel mailing list