Submitter | Pierre-Yves David |
---|---|
Date | Dec. 2, 2014, 8:32 p.m. |
Message ID | <0e50f872225ac6082796.1417552342@marginatus.alto.octopoid.net> |
Download | mbox | patch |
Permalink | /patch/6958/ |
State | Superseded |
Headers | show |
Comments
On 12/02/2014 09:32 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1417542380 28800 > # Tue Dec 02 09:46:20 2014 -0800 > # Node ID 0e50f872225ac6082796871f236c05c2e51a4d25 > # Parent 6b2953028561f9b49fb79e975d68e85e8666f13a > rebase: add a 'revtodo' constant to to what? > The state mapping is using '-1' to mark revision that have not been rebased yet. > We introduce and use a constant for that purpose. This will help emphasising the > fact the value mean something else than nullrev. > > diff --git a/hgext/rebase.py b/hgext/rebase.py > --- a/hgext/rebase.py > +++ b/hgext/rebase.py > @@ -808,11 +809,11 @@ def inrebase(repo, originalwd, state): > > return False > > def abort(repo, originalwd, target, state): > 'Restore the repository to its original state' > - dstates = [s for s in state.values() if s > nullrev] > + dstates = [s for s in state.values() if s > revtodo] I am not sure revtodo is more correct here than nullrev. If you don't want to rely on nullrev=0-1 then I think it would be more correct to use >= 0 to filter out all normal revision numbers. That is also what is used in clearrebased. > @@ -831,11 +832,11 @@ def abort(repo, originalwd, target, stat > # Update away from the rebase if necessary > if inrebase(repo, originalwd, state): > merge.update(repo, repo[originalwd].rev(), False, True, False) > > # Strip from the first rebased revision > - rebased = filter(lambda x: x > -1 and x != target, state.values()) > + rebased = filter(lambda x: x > revtodo and x != target, state.values()) Again, when we don't rely on revtodo==nullrev=-1, wouldn't it be more correct to use >= 0? /Mads
On 12/02/2014 05:02 PM, Mads Kiilerich wrote: > On 12/02/2014 09:32 PM, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1417542380 28800 >> # Tue Dec 02 09:46:20 2014 -0800 >> # Node ID 0e50f872225ac6082796871f236c05c2e51a4d25 >> # Parent 6b2953028561f9b49fb79e975d68e85e8666f13a >> rebase: add a 'revtodo' constant to > > to what? > >> The state mapping is using '-1' to mark revision that have not been >> rebased yet. >> We introduce and use a constant for that purpose. This will help >> emphasising the >> fact the value mean something else than nullrev. >> >> diff --git a/hgext/rebase.py b/hgext/rebase.py >> --- a/hgext/rebase.py >> +++ b/hgext/rebase.py > > >> @@ -808,11 +809,11 @@ def inrebase(repo, originalwd, state): >> return False >> def abort(repo, originalwd, target, state): >> 'Restore the repository to its original state' >> - dstates = [s for s in state.values() if s > nullrev] >> + dstates = [s for s in state.values() if s > revtodo] > > I am not sure revtodo is more correct here than nullrev. If you don't > want to rely on nullrev=0-1 then I think it would be more correct to use > >= 0 to filter out all normal revision numbers. That is also what is > used in clearrebased. Good point, fixed it in a V2.
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -21,10 +21,11 @@ from mercurial.commands import templateo from mercurial.node import nullrev from mercurial.lock import release from mercurial.i18n import _ import os, errno +revtodo = -1 nullmerge = -2 revignored = -3 cmdtable = {} command = cmdutil.command(cmdtable) @@ -365,11 +366,11 @@ def rebase(ui, repo, **opts): sortedstate = sorted(state) total = len(sortedstate) pos = 0 for rev in sortedstate: pos += 1 - if state[rev] == -1: + if state[rev] == revtodo: ui.progress(_("rebasing"), pos, ("%d:%s" % (rev, repo[rev])), _('changesets'), total) p1, p2 = defineparents(repo, rev, target, state, targetancestors) storestatus(repo, originalwd, target, state, collapsef, keepf, @@ -781,11 +782,11 @@ def restorestatus(repo): skipped = set() # recompute the set of skipped revs if not collapse: seen = set([target]) for old, new in sorted(state.items()): - if new != nullrev and new in seen: + if new != revtodo and new in seen: skipped.add(old) seen.add(new) repo.ui.debug('computed skipped revs: %s\n' % (' '.join(str(r) for r in sorted(skipped)) or None)) repo.ui.debug('rebase status resumed\n') @@ -808,11 +809,11 @@ def inrebase(repo, originalwd, state): return False def abort(repo, originalwd, target, state): 'Restore the repository to its original state' - dstates = [s for s in state.values() if s > nullrev] + dstates = [s for s in state.values() if s > revtodo] immutable = [d for d in dstates if not repo[d].mutable()] cleanup = True if immutable: repo.ui.warn(_("warning: can't clean up immutable changesets %s\n") % ', '.join(str(repo[r]) for r in immutable), @@ -831,11 +832,11 @@ def abort(repo, originalwd, target, stat # Update away from the rebase if necessary if inrebase(repo, originalwd, state): merge.update(repo, repo[originalwd].rev(), False, True, False) # Strip from the first rebased revision - rebased = filter(lambda x: x > -1 and x != target, state.values()) + rebased = filter(lambda x: x > revtodo and x != target, state.values()) if rebased: strippoints = [c.node() for c in repo.set('roots(%ld)', rebased)] # no backup of rebased cset versions needed repair.strip(repo.ui, repo, strippoints) @@ -873,11 +874,11 @@ def buildstate(repo, dest, rebaseset, co if not collapse and samebranch and root in dest.children(): repo.ui.debug('source is a child of destination\n') return None repo.ui.debug('rebase onto %d starting from %s\n' % (dest, root)) - state.update(dict.fromkeys(rebaseset, nullrev)) + state.update(dict.fromkeys(rebaseset, revtodo)) # Rebase tries to turn <dest> into a parent of <root> while # preserving the number of parents of rebased changesets: # # - A changeset with a single parent will always be rebased as a # changeset with a single parent. @@ -1010,11 +1011,11 @@ def summaryhook(ui, repo): except error.RepoLookupError: # i18n: column positioning for "hg summary" msg = _('rebase: (use "hg rebase --abort" to clear broken state)\n') ui.write(msg) return - numrebased = len([i for i in state.itervalues() if i != -1]) + numrebased = len([i for i in state.itervalues() if i != revtodo]) # i18n: column positioning for "hg summary" ui.write(_('rebase: %s, %s (rebase --continue)\n') % (ui.label(_('%d rebased'), 'rebase.rebased') % numrebased, ui.label(_('%d remaining'), 'rebase.remaining') % (len(state) - numrebased)))