Patchwork [1,of,3] rebase: add a 'revtodo' constant to

login
register
mail settings
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

Pierre-Yves David - Dec. 2, 2014, 8:32 p.m.
# 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

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.
Mads Kiilerich - Dec. 3, 2014, 1:02 a.m.
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
Pierre-Yves David - Dec. 4, 2014, 3:15 p.m.
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)))