D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz abandoned this revision. martinvonz added a comment. I think I've now convinced myself that this won't work. Consider these statements: 1. A revset should resolve to the same revisions whatever command it's passed to 2. `hg co X` should check out the "best" node with name X 3. `hg co X^` should check out the parent of the commit checked out by `hg co X` 4. `hg log -r X` should include all commits in X It doesn't seem to me like we can have all of them. We currently have 1, 2, and 3. I think I originally would have made it so we had 1, 3, and 4 (because `singlenode()` picked the highest revision returned from `namemap()`). Then I sent https://phab.mercurial-scm.org/D3852 because I wanted to preserve 2, but then I realized that that would instead break 3 (because it would be calling `singlenode()` directly from `revsingle()`, but calling `namemap()` from the revset code). I really don't want to break 1. We could break 2 by requiring a revset to have exactly one revision in `revsingle()`. Imagine we have a `best(name)` revset that resolved to the best node given the name (so e.g. `best(stable)` would find the highest revision on unclosed heads of the stable branch). One would then have to do `hg co 'best(stable)'`, or we could use a `~` suffix (as Yuya suggested for the opposite thing). I think that sounds pretty reasonable, but we obviously can't do that because of BC. Therefore, my conclusion is that we'll have to live with breaking 4 (as we already do), so I'll abandon this patch. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: yuja, durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
yuja added a comment. > I was also thinking we could do that, but it feels like a workaround to me. I think of bookmarks and tags as pointing to specific commits. I think of branches as being many commits. `hg help revisions` says this: > > Any other string is treated as a bookmark, tag, or branch name. A bookmark > is a movable pointer to a revision. A tag is a permanent name associated > with a revision. A branch name denotes the tipmost open branch head of > that branch - or if they are all closed, the tipmost closed head of the > branch. Bookmark, tag, and branch names must not contain the ":" > character. > > It specifically mentions bookmarks, tags, and branches, and it describes how each name resolves to a commit. To me, that makes it sound like some symbols can resolve to multiple commits. And indeed, revset aliases can already resolve to multiple commits. I read it implying that any sort of symbol resolves to a single revision since all three core "names" work in such way. I'm not against adding multi-node symbol resolution because it seems useful, but I agree with Sean that it would introduce inconsistency. I have no idea about the `namemap()` API. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: yuja, durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
> I was also thinking we could do that, but it feels like a workaround to me. > I think of bookmarks and tags as pointing to specific commits. I think of > branches as being many commits. `hg help revisions` says this: > > Any other string is treated as a bookmark, tag, or branch name. A bookmark > is a movable pointer to a revision. A tag is a permanent name associated > with a revision. A branch name denotes the tipmost open branch head of > that branch - or if they are all closed, the tipmost closed head of the > branch. Bookmark, tag, and branch names must not contain the ":" > character. > > It specifically mentions bookmarks, tags, and branches, and it describes > how each name resolves to a commit. To me, that makes it sound like some > symbols can resolve to multiple commits. And indeed, revset aliases can > already resolve to multiple commits. I read it implying that any sort of symbol resolves to a single revision since all three core "names" work in such way. I'm not against adding multi-node symbol resolution because it seems useful, but I agree with Sean that it would introduce inconsistency. I have no idea about the `namemap()` API. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
yuja added a comment. > > 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. How about adding syntax to resolve multinode namespace symbols? `stable~`, for example, will be resolved to all revisions in the stable branch. Implementation wise, we can get rid of the `revsymbol()` hack, and extra `repo[n]` lookup won't be needed. A possible drawback (other than the '~' suffix itself) is we'll soon run out of shell-safe symbols. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: yuja, durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
> > 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. How about adding syntax to resolve multinode namespace symbols? `stable~`, for example, will be resolved to all revisions in the stable branch. Implementation wise, we can get rid of the `revsymbol()` hack, and extra `repo[n]` lookup won't be needed. A possible drawback (other than the '~' suffix itself) is we'll soon run out of shell-safe symbols. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz (Martin von Zweigbergk) writes: > martinvonz added a comment. > > > In https://phab.mercurial-scm.org/D3715#58729, @smf wrote: > > > --=-=-= > > Content-Type: text/plain > > > > martinvonz (Martin von Zweigbergk) writes: > > > > > martinvonz added a comment. > > > > > > In https://phab.mercurial-scm.org/D3715#58720, @smf wrote: > > > > > > > Sean Farley writes: > > > > > > > > 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. > > > > No, I understand what you're trying to do. I was attempting to paint a > > picture of what it looks like to me in the sense of "this works for one > > type of object but not this other one". Why? Why does it work for topics > > and not branches? As a user, and believe me I have years of experience > > with angry ones, they do not understand the nuanced differences between > > branches and topics. They just lump them all together. > > > As I said in the commit message, I'm aware of this argument, I just > disagree. I think most users have never even tried to do `hg log -r default` > and that's the only shipped-with-core namespace that behaves funny (IMO). > > > Topics is going out of its way to pretend they are in the same namespace > > as branches (that's the motivation of my mini rant in my previous > > message). The difference between branches and topics is lost on everyone > > outside this thread. When I tried to introduce them onto Bitbucket I > > only got headaches: "can I merge between a topic and branch? can I merge > > between a topic on branch A and another topic on branch B? if so, what > > does that mean? Why is branch B still open? I merged one topic into > > another and it closed the three topics it was based on, why?" > > > > It solidified my stance that there should only be one namespace for most > > users: branches. > > Sounds more like a criticism of topics. > > > And yes, your patch only applies to topics. > > *Maybe* to topics. That will be up to that extensions developers. > > > My point is the cognitive > > load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the > > other? To the average hg user, both topics and branches are treated > > (semantically) the same. Why one and not the other? > > For historical reasons (`hg log -r branch-bar` is protected by BC). Again, > I think few users have even tried that command. If they did, perhaps they did > it hoping that it would list all the commits on the branch? > > > Let's say you and I say a repo. I add a branch (because that's the only > > thing I use) and you add a topic. Cool. Another dev comes along. 'hg log > > -r martins-work' lists all the commits of his feature work, but, 'hg log > > -r seans-work' only lists the tip-most commit of my feature work. Why? > > Why should that dev be tripped up by this? > > See my commit message. > > > Furthermore, what about the revset language? Should 'topic-foo' become > > 'topic(topic-foo)' in our AST? > > No. The way I did it in this patch was to make the symbol "topic-foo" > itself resolve to multiple revisions. > > > I was hoping the abstraction I made > > between branches and topics was clear but I guess I am too terse > > sometimes, so I apologize. > > I think I see the similarities between them (and I did before your message > too :)). > > > > > > >> > "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
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz added a comment. In https://phab.mercurial-scm.org/D3715#58729, @smf wrote: > --=-=-= > Content-Type: text/plain > > martinvonz (Martin von Zweigbergk) writes: > > > martinvonz added a comment. > > > > In https://phab.mercurial-scm.org/D3715#58720, @smf wrote: > > > > > Sean Farley writes: > > > > > > 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. > > No, I understand what you're trying to do. I was attempting to paint a > picture of what it looks like to me in the sense of "this works for one > type of object but not this other one". Why? Why does it work for topics > and not branches? As a user, and believe me I have years of experience > with angry ones, they do not understand the nuanced differences between > branches and topics. They just lump them all together. As I said in the commit message, I'm aware of this argument, I just disagree. I think most users have never even tried to do `hg log -r default` and that's the only shipped-with-core namespace that behaves funny (IMO). > Topics is going out of its way to pretend they are in the same namespace > as branches (that's the motivation of my mini rant in my previous > message). The difference between branches and topics is lost on everyone > outside this thread. When I tried to introduce them onto Bitbucket I > only got headaches: "can I merge between a topic and branch? can I merge > between a topic on branch A and another topic on branch B? if so, what > does that mean? Why is branch B still open? I merged one topic into > another and it closed the three topics it was based on, why?" > > It solidified my stance that there should only be one namespace for most > users: branches. Sounds more like a criticism of topics. > And yes, your patch only applies to topics. *Maybe* to topics. That will be up to that extensions developers. > My point is the cognitive > load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the > other? To the average hg user, both topics and branches are treated > (semantically) the same. Why one and not the other? For historical reasons (`hg log -r branch-bar` is protected by BC). Again, I think few users have even tried that command. If they did, perhaps they did it hoping that it would list all the commits on the branch? > Let's say you and I say a repo. I add a branch (because that's the only > thing I use) and you add a topic. Cool. Another dev comes along. 'hg log > -r martins-work' lists all the commits of his feature work, but, 'hg log > -r seans-work' only lists the tip-most commit of my feature work. Why? > Why should that dev be tripped up by this? See my commit message. > Furthermore, what about the revset language? Should 'topic-foo' become > 'topic(topic-foo)' in our AST? No. The way I did it in this patch was to make the symbol "topic-foo" itself resolve to multiple revisions. > I was hoping the abstraction I made > between branches and topics was clear but I guess I am too terse > sometimes, so I apologize. I think I see the similarities between them (and I did before your message too :)). > > >> > "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. > > Right, but as I tried to explain in my previous email I believe your > extension should add another custom flag of '--phab https://phab.mercurial-scm.org/D3715' to signify > that "I want all the commits
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
--=-=-= Content-Type: text/plain martinvonz (Martin von Zweigbergk) writes: > martinvonz added a comment. > > > In https://phab.mercurial-scm.org/D3715#58720, @smf wrote: > > > Sean Farley writes: > > > > > durin42 (Augie Fackler) 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. No, I understand what you're trying to do. I was attempting to paint a picture of what it looks like to me in the sense of "this works for one type of object but not this other one". Why? Why does it work for topics and not branches? As a user, and believe me I have years of experience with angry ones, they do not understand the nuanced differences between branches and topics. They just lump them all together. Topics is going out of its way to pretend they are in the same namespace as branches (that's the motivation of my mini rant in my previous message). The difference between branches and topics is lost on everyone outside this thread. When I tried to introduce them onto Bitbucket I only got headaches: "can I merge between a topic and branch? can I merge between a topic on branch A and another topic on branch B? if so, what does that mean? Why is branch B still open? I merged one topic into another and it closed the three topics it was based on, why?" It solidified my stance that there should only be one namespace for most users: branches. And yes, your patch only applies to topics. My point is the cognitive load. Why is it '-r topic-foo' for one and not '-r branch-bar' for the other? To the average hg user, both topics and branches are treated (semantically) the same. Why one and not the other? Let's say you and I say a repo. I add a branch (because that's the only thing I use) and you add a topic. Cool. Another dev comes along. 'hg log -r martins-work' lists all the commits of his feature work, but, 'hg log -r seans-work' only lists the tip-most commit of my feature work. Why? Why should that dev be tripped up by this? Furthermore, what about the revset language? Should 'topic-foo' become 'topic(topic-foo)' in our AST? I was hoping the abstraction I made between branches and topics was clear but I guess I am too terse sometimes, so I apologize. > > "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. Right, but as I tried to explain in my previous email I believe your extension should add another custom flag of '--phab D3715' to signify that "I want all the commits
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
smf added a comment. It seems I'm having email sending trouble ... going to attempt to send again REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
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 writes:
> >
> > > durin42 (Augie Fackler) 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
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz added a comment. In https://phab.mercurial-scm.org/D3715#58720, @smf wrote: > Sean Farley writes: > > > durin42 (Augie Fackler) 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
Sean Farley writes: > durin42 (Augie Fackler) 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. In terms of revsets, this means: hg log -r 'default' -> hg log -r 'branch(default)' I'm using branches to illustrate the argument in terms of core behavior but will go back to topics further down. But what about more complex revsets? 'default & draft()' will now mean 'branch(default) & draft()' which is something completely different. You could argue that, "Oh, well, we'll only do that if the argument sent to -r is equal to a single branch name." But what about users that expect '-r NAME' to work like 'branch(NAME)' in a revset? I would expect they learn a bad habit of 'default == branch(default)'. And what about a more complex that will reduce down to 'default'? Will that be 'branch(default)' now? It's not just about 'hg log,' though. What about 'hg push -r default'? Currently, this will push the highest rev of default but with default being replaced in a user's mental model with 'branch(default)' this should push each head. For these reasons, I am fairly against having a conditional hack like this on top of -r. I'd like to point out that this is why -b exists. 'hg log -b default' is the command you're looking for. "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). P.S. (personal opinion mini-rant) This is why I don't really like the concept of topics. In reality, they are conceptually sub-branches. What I really want 99% of the time is a branch (we call them named branches) that is closed when merged into default / stable. There are many points I made about this almost a year ago but the biggest to me are: core hg commands work out of the box, bitbucket knows how to deal with them, and old clients know what to do with 90% of the cases. I do think topics have a place in an advanced workflow but not as feature branches. /endrant signature.asc Description: PGP signature ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz added a comment. In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote: > 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? In https://phab.mercurial-scm.org/D3715#58507, @lothiraldan wrote: > In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote: > > > 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? > > > The proposition of making namespaces behave differently than branches, and potentially making two namespaces behave differently (one with `multinode=True` and the other with `multinode=False`) make my spider sense tickle. But I will take time to see the exact implications of that series. As I said in the commit message, I think branches should have multinode=True, but we can't do that for BC reasons. Bookmarks and tags clearly identify a single commit per name (at least in my mind). How about we introduce the multinode option and default it to False for now, but plan to change the default to True later and then let only branches have the old behavior (i.e. multinode=False) and discourage extensions from setting multinode=False? That would result in consistency between namespaces (except for the legacy case of branches) while giving existing extensions some time to adapt (perhaps by making namemap return at most one node if they view their names as identifying a single commit). Btw, the topics extension was just an example. We have an internal extension that's the reason for this patch. I don't use topics, so I don't care too much if topics decide to have one commit per name or not (but if I did use topics, I think I would definitely view it as having multiple nodes per name). REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
lothiraldan added a comment. In https://phab.mercurial-scm.org/D3715#58383, @durin42 wrote: > 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? The proposition of making namespaces behave differently than branches, and potentially making two namespaces behave differently (one with `multinode=True` and the other with `multinode=False`) make my spider sense tickle. But I will take time to see the exact implications of that series. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
durin42 (Augie Fackler) 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. 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.) signature.asc Description: PGP signature ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
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? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3715 To: martinvonz, #hg-reviewers, durin42 Cc: durin42, smf, lothiraldan, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3715: namespaces: allow namespaces whose symbols resolve to many nodes (API)
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
The goal of this commit is to allow namespaces that support multiple
nodes per name to also have the name resolve to those multiple nodes
in the revset. For example, the "topics" namespace seems to be a
natural candidate for this (I don't know if the extension's maintainer
agrees). I view a topic as having multiple nodes and I would therefore
expect `hg log -r my-topic` to list all those nodes. Note that the
topics extension already supports multiple nodes per name, but we
don't expose that to the user in a consistent way (each namespace has
to define its own revset).
This commit adds an option to namespaces to indicate that `hg log -r
` and similar should resolve to the nodes that
the namespace says and not just the highest revnum among them.
Marked (API) because I repurposed singlenode() to nodes().
Note: I think branches should also resolve to multiple nodes (so
e.g. `hg log -r stable` lists all nodes on stable), but it's obviously
too late to change that now (and perhaps BC is the reason it even
behaves the way it does). I also realize that any namespace that were
to use the new mechanism would be inconsistent with how branches
work. I think the convenience and intuitiveness outweighs the cost of
that inconsistency.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3715
AFFECTED FILES
mercurial/namespaces.py
mercurial/revset.py
mercurial/scmutil.py
tests/paritynamesext.py
tests/test-revset2.t
CHANGE DETAILS
diff --git a/tests/test-revset2.t b/tests/test-revset2.t
--- a/tests/test-revset2.t
+++ b/tests/test-revset2.t
@@ -675,6 +675,12 @@
6
$ log 'named("tags")'
6
+ $ hg --config extensions.revnamesext=$TESTDIR/paritynamesext.py log
--template '{rev}\n' -r even
+ 0
+ 2
+ 4
+ 6
+ 8
issue2437
diff --git a/tests/paritynamesext.py b/tests/paritynamesext.py
new file mode 100644
--- /dev/null
+++ b/tests/paritynamesext.py
@@ -0,0 +1,26 @@
+# Defines a namespace with names 'even' and 'odd'
+
+from __future__ import absolute_import
+
+from mercurial import (
+namespaces,
+)
+
+def reposetup(ui, repo):
+def namemap(repo, name):
+if name == 'even':
+parity = 0
+elif name == 'odd':
+parity = 1
+else:
+return []
+return [repo[rev].node() for rev in repo if rev % 2 == parity]
+def nodemap(repo, node):
+return [repo[node].rev() % 2]
+
+ns = namespaces.namespace(b'parity', templatename=b'parity',
+ logname=b'parity',
+ listnames=lambda r: ['even', 'odd'],
+ namemap=namemap, nodemap=nodemap,
+ multinode=True)
+repo.names.addnamespace(ns)
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -488,7 +488,7 @@
except error.RepoLookupError:
return False
-def _revsymbol(repo, symbol):
+def _revsymbol(repo, symbol, allowmultiple):
if not isinstance(symbol, bytes):
msg = ("symbol (%s of type %s) was not a string, did you mean "
"repo[symbol]?" % (symbol, type(symbol)))
@@ -524,9 +524,9 @@
# look up bookmarks through the name interface
try:
-node = repo.names.singlenode(repo, symbol)
-rev = repo.changelog.rev(node)
-return [repo[rev]]
+nodes = repo.names.nodes(repo, symbol, allowmultiple)
+revs = [repo.changelog.rev(n) for n in nodes]
+return [repo[r2] for r2 in revs] # "r2" to keep pyflakes happy
except KeyError:
pass
@@ -550,10 +550,19 @@
i.e. things like ".", "tip", "1234", "deadbeef", "my-bookmark" work, but
not "max(public())".
"""
-ctxs = _revsymbol(repo, symbol)
+ctxs = _revsymbol(repo, symbol, allowmultiple=False)
assert len(ctxs) == 1
return ctxs[0]
+def revsymbolmultiple(repo, symbol):
+"""Returns all contexts given a single revision symbol (as string).
+
+This is similar to revsymbol(), but if "symbol" is a namespace symbol,
+then this may return many context (if the namespace maps the symbol to
+many revisions).
+"""
+return _revsymbol(repo, symbol, allowmultiple=True)
+
def _filterederror(repo, changeid):
"""build an exception to be raised about a filtered changeid
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -118,11 +118,14 @@
def stringset(repo, subset, x, order):
if not x:
raise error.ParseError(_("empty string is not a valid revision"))
-x = scmutil.intrev(scmutil.revsymbol(repo, x))
-if (x in subset
-or x == node.nullrev and isinstance(subset, fullreposet)):
-return
