Patchwork [4,of,8] rebase: move no rebasestate file check out of restorestatus

login
register
mail settings
Submitter Christian Delahousse
Date Oct. 16, 2015, 2:33 a.m.
Message ID <ef6cee42f6058ae82497.1444962813@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11136/
State Changes Requested
Headers show

Comments

Christian Delahousse - Oct. 16, 2015, 2:33 a.m.
# HG changeset patch
# User Christian Delahousse <cdelahousse@fb.com>
# Date 1444948508 25200
#      Thu Oct 15 15:35:08 2015 -0700
# Node ID ef6cee42f6058ae824978db8559920dd6c1e3fb2
# Parent  d8c393e7c3bec8f2ba0ea286d121c05a999e7ade
rebase: move no rebasestate file check out of restorestatus

Throwing exceptions when no statefile exists is not very exceptional. I change
restorestatus so it returns None instead of throwing an exception. This also
allows the error handling (no rebase running with --continue) to be moved up one
level of abstraction with the majority of the other error handling code.
Matt Mackall - Oct. 16, 2015, 10:43 p.m.
On Thu, 2015-10-15 at 19:33 -0700, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1444948508 25200
> #      Thu Oct 15 15:35:08 2015 -0700
> # Node ID ef6cee42f6058ae824978db8559920dd6c1e3fb2
> # Parent  d8c393e7c3bec8f2ba0ea286d121c05a999e7ade
> rebase: move no rebasestate file check out of restorestatus
> 
> Throwing exceptions when no statefile exists is not very exceptional.

Well.. it has the nice property that we always know what type to expect
when restorestatus returns. Here you've made it "nullable" without
updating all the callers to expect None. So now we'll get a weird tuple
unpacking traceback on one path. And you'll have to duplicate the
exception message.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -258,8 +258,12 @@ 
                 ui.warn(_('tool option will be ignored\n'))
 
             try:
+                result = restorestatus(repo)
+                if not result:
+                    raise error.Abort(_('no rebase in progress'))
+
                 (originalwd, target, state, skipped, collapsef, keepf,
-                 keepbranchesf, external, activebookmark) = restorestatus(repo)
+                 keepbranchesf, external, activebookmark) = result
             except error.RepoLookupError:
                 if abortf:
                     clearstatus(repo)
@@ -885,7 +889,8 @@ 
     except IOError as err:
         if err.errno != errno.ENOENT:
             raise
-        raise error.Abort(_('no rebase in progress'))
+        # No file, so no rebase in progress
+        return None
 
     if keepbranches is None:
         raise error.Abort(_('.hg/rebasestate is incomplete'))