D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In https://phab.mercurial-scm.org/D3715#58726, @martinvonz wrote:
  
  > In https://phab.mercurial-scm.org/D3715#58720, @smf wrote:
  >
  > > Sean Farley <sean at farley.io> writes:
  > >
  > > > durin42 (Augie Fackler) <phabricator at mercurial-scm.org> writes:
  > > > 
  > > >> durin42 added subscribers: lothiraldan, smf, durin42.
  > > >>  durin42 accepted this revision as: durin42.
  > > >>  durin42 added a comment.
  > > >> 
  > > >>   I'm in favor, but feel like I've got enough conflict of interest I shouldn't land the patches.
  > > >>   
  > > >>   @smf @lothiraldan this might be of interest to both of you?
  > > > 
  > > > Side note: I keep missing messages that I'm tagged in because I'm not
  > > >  explicitly mentioned in a CC field. Is it possible to add a CC to each
  > > >  person tagged in a message?
  > > > 
  > > > Side note2: Phabricator emails are really non-trivial to parse and
  > > >  (worse!) search. The raw emails are not simple, raw text so I'm having
  > > >  trouble tagging these for higher priority.
  > >
  > > I think I got this ironed out now.
  > >
  > > > Thanks for alerting me of this series! I've had a discussion with Martin
  > > >  about this on IRC but I'm a bit out of time today to respond (but
  > > >  definitely do want to respond). (I'm going to try to spend some time in
  > > >  the mornings to do my email triaging so I can get back on top of this
  > > >  list.)
  > >
  > > This patch strikes me as a Seems-Like-A-Good-Idea-But-Could-Blowup type
  > >  of thing. So, what this patch does is conditionally change the behavior
  > >  of 'log -r' based on the type of object passed in.
  >
  >
  > No, it just allows namespaces to do that. As I said (or tried to say) in the commit message, `hg log -r stable` is protected by BC, so we can't change it. There should be no functional change from this patch.
  >
  > Unfortunately, I think much of your comments below were based on this incorrect assumption about what this patch does. I've read through it, but it doesn't seem very relevant.
  >
  > > "But what about topics?" you ask. Well,
  > >  personally, I think that extension should add -t to log if it wants that
  > >  functionality (-t is available to both 'push' and 'log' for those
  > >  curious).
  >
  > This is closer to the actual point of this patch. As I tried to say in the commit message, this patch allows an extension to indicate that the name->nodes mapping it provides should present in the plain revset symbol. For example, if the topics extension opts in to this behavior, then `hg log -r my-topic` would start including all nodes in the topic. More important to me (I don't use topics) is that our Google-internal extension could opt in (it would kind of lets you do something like `hg log -r D3715` and get current and past versions of https://phab.mercurial-scm.org/D3715). Again, I don't intend to change how `hg log -r stable` behaves.
  
  
  Also, I said in the commit message that I view a topic as a set of commits. Namespaces allows modeling that (as I'm sure you know). The topics extension takes advantage of that and does model it like that (i.e. by letting namemap() map a topic name to many nodes). It's just that the revset code resolves the symbol to a single value. To help illustrate what I mean, I find it a little silly that the topic extension tests pass with this patch applied:
  
  diff --git a/hgext3rd/topic/__init__.py b/hgext3rd/topic/__init__.py
  
  - a/hgext3rd/topic/__init__.py
  
  +++ b/hgext3rd/topic/__init__.py
  @@ -284,7 +284,7 @@ def _namemap(repo, name):
  
    if name not in repo.topics:
        return []
    node = repo.changelog.node
  
  - return [node(rev) for rev in repo.revs('topic(%s)', name)]
  
  +    return [node(rev) for rev in repo.revs('max(topic(%s))', name)]
  
  def _nodemap(repo, node):
  
    ctx = repo[node]

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, durin42
Cc: durin42, smf, lothiraldan, mercurial-devel