| Submitter | Boris Feld |
|---|---|
| Date | Sept. 27, 2018, 5:08 p.m. |
| Message ID | <79a0f8fcb5c1af9a8ad5.1538068119@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/35147/ |
| State | Accepted |
| Headers | show |
Comments
On Thu, 27 Sep 2018 19:08:39 +0200, Boris Feld wrote: > # HG changeset patch > # User Boris Feld <boris.feld@octobus.net> > # Date 1537998671 -7200 > # Wed Sep 26 23:51:11 2018 +0200 > # Node ID 79a0f8fcb5c1af9a8ad5dd292a8ef67d75b6779d > # Parent 77b0a61c3cc5be70c3686f9b7217e59fbf98f5c7 > # EXP-Topic trackfold > # Available At https://bitbucket.org/octobus/mercurial-devel/ > # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 79a0f8fcb5c1 > rebase: use tuple as `replacement` keys > > Now that `cleanupnodes` support tuples as key, we update the rebase code to use > them. No changes in the replacement tracked are introduced yet. > > diff --git a/hgext/rebase.py b/hgext/rebase.py > --- a/hgext/rebase.py > +++ b/hgext/rebase.py > @@ -1777,15 +1777,16 @@ def clearrebased(ui, repo, destmap, stat > else: > succs = (newnode,) > if succs is not None: > - replacements[oldnode] = succs > + replacements[(oldnode,)] = succs Somewhat related to this. Is there any reason to build replacements as a dict? I don't think we'll ever want to lookup succs by multi-oldnodes.
On 30/09/2018 14:34, Yuya Nishihara wrote: > On Thu, 27 Sep 2018 19:08:39 +0200, Boris Feld wrote: >> # HG changeset patch >> # User Boris Feld <boris.feld@octobus.net> >> # Date 1537998671 -7200 >> # Wed Sep 26 23:51:11 2018 +0200 >> # Node ID 79a0f8fcb5c1af9a8ad5dd292a8ef67d75b6779d >> # Parent 77b0a61c3cc5be70c3686f9b7217e59fbf98f5c7 >> # EXP-Topic trackfold >> # Available At https://bitbucket.org/octobus/mercurial-devel/ >> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 79a0f8fcb5c1 >> rebase: use tuple as `replacement` keys >> >> Now that `cleanupnodes` support tuples as key, we update the rebase code to use >> them. No changes in the replacement tracked are introduced yet. >> >> diff --git a/hgext/rebase.py b/hgext/rebase.py >> --- a/hgext/rebase.py >> +++ b/hgext/rebase.py >> @@ -1777,15 +1777,16 @@ def clearrebased(ui, repo, destmap, stat >> else: >> succs = (newnode,) >> if succs is not None: >> - replacements[oldnode] = succs >> + replacements[(oldnode,)] = succs > Somewhat related to this. Is there any reason to build replacements as a dict? > I don't think we'll ever want to lookup succs by multi-oldnodes. I don't think so, using a dict is probably an artifact of when this code belonged to rebase or histedit. Here, a dict was probably used to ease transitive update of finals successors. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Wed, 3 Oct 2018 16:56:56 +0200, Boris FELD wrote: > On 30/09/2018 14:34, Yuya Nishihara wrote: > > On Thu, 27 Sep 2018 19:08:39 +0200, Boris Feld wrote: > >> # HG changeset patch > >> # User Boris Feld <boris.feld@octobus.net> > >> # Date 1537998671 -7200 > >> # Wed Sep 26 23:51:11 2018 +0200 > >> # Node ID 79a0f8fcb5c1af9a8ad5dd292a8ef67d75b6779d > >> # Parent 77b0a61c3cc5be70c3686f9b7217e59fbf98f5c7 > >> # EXP-Topic trackfold > >> # Available At https://bitbucket.org/octobus/mercurial-devel/ > >> # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 79a0f8fcb5c1 > >> rebase: use tuple as `replacement` keys > >> > >> Now that `cleanupnodes` support tuples as key, we update the rebase code to use > >> them. No changes in the replacement tracked are introduced yet. > >> > >> diff --git a/hgext/rebase.py b/hgext/rebase.py > >> --- a/hgext/rebase.py > >> +++ b/hgext/rebase.py > >> @@ -1777,15 +1777,16 @@ def clearrebased(ui, repo, destmap, stat > >> else: > >> succs = (newnode,) > >> if succs is not None: > >> - replacements[oldnode] = succs > >> + replacements[(oldnode,)] = succs > > Somewhat related to this. Is there any reason to build replacements as a dict? > > I don't think we'll ever want to lookup succs by multi-oldnodes. > I don't think so, using a dict is probably an artifact of when this code > belonged to rebase or histedit. Here, a dict was probably used to ease > transitive update of finals successors. It did make some sense before because oldnode was a single node. My point is that the use of the dict brings unnecessary restriction to the key type, which has to be hashable so can't be a list. That said, I don't care much about this.
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -1777,15 +1777,16 @@ def clearrebased(ui, repo, destmap, stat else: succs = (newnode,) if succs is not None: - replacements[oldnode] = succs + replacements[(oldnode,)] = succs scmutil.cleanupnodes(repo, replacements, 'rebase', moves, backup=backup) if fm: hf = fm.hexfunc fl = fm.formatlist fd = fm.formatdict changes = {} - for oldn, newn in replacements.iteritems(): - changes[hf(oldn)] = fl([hf(n) for n in newn], name='node') + for oldns, newn in replacements.iteritems(): + for oldn in oldns: + changes[hf(oldn)] = fl([hf(n) for n in newn], name='node') nodechanges = fd(changes, key="oldnode", value="newnodes") fm.data(nodechanges=nodechanges)