Patchwork [2,of,8] rebase: during abort add warning to user on any exception

login
register
mail settings
Submitter Christian Delahousse
Date Oct. 16, 2015, 2:33 a.m.
Message ID <1dd6fe8f95da7735de74.1444962811@dev4253.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11134/
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 1444944088 25200
#      Thu Oct 15 14:21:28 2015 -0700
# Node ID 1dd6fe8f95da7735de7485d5145a2c389860e387
# Parent  c3787f7156bdbb6d5f7c6e2540248b911162c0b3
rebase: during abort add warning to user on any exception

If an exception happens during an --abort, the user should be notified to check
their repo. This patch will do so by catching, warning the user, and then
rethrowing.
Matt Mackall - Oct. 17, 2015, 7:26 p.m.
On Thu, 2015-10-15 at 19:33 -0700, Christian Delahousse wrote:
> # HG changeset patch
> # User Christian Delahousse <cdelahousse@fb.com>
> # Date 1444944088 25200
> #      Thu Oct 15 14:21:28 2015 -0700
> # Node ID 1dd6fe8f95da7735de7485d5145a2c389860e387
> # Parent  c3787f7156bdbb6d5f7c6e2540248b911162c0b3
> rebase: during abort add warning to user on any exception
> 
> If an exception happens during an --abort, the user should be notified to check
> their repo. This patch will do so by catching, warning the user, and then
> rethrowing.
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -963,6 +963,11 @@
>          if activebookmark and activebookmark in repo._bookmarks:
>              bookmarks.activate(repo, activebookmark)
>  
> +    except Exception:
> +        repo.ui.warn(_('warning: encountered an exception during rebase '
> +            '--abort; the repository may not have been completely '
> +            'cleaned up\n'))

This is quite verbose but also not very prescriptive. Users are very
reluctant to read/comprehend _even just one line error messages_, so if
we're going to add another line, it really needs to tell them what to
do. I'm sure we've both seen git help requests that quote a long error
message and say "now what??" and the answer is to quote the error
message back at them verbatim and they reply "thanks!".

I don't know that we actually have a good generic piece of advice to
give them here. But I think we can reduce this to simply:

 $ hg rebase --abort
 warning: clean-up interrupted, check your repository state!
 abort: <thing that went wrong>

It'd actually be better if we could promote the warning into an abort
hint= parameter though:

 $ hg rebase --abort
 abort: <thing that went wrong>
 (clean-up interrupted, check your repository state!)

The parens are a really good hint to users to actually read the previous
line to find the actual error message... while still giving them some
advice.

We could make a helper to do that where possible in cmdutil:

def warnorhint(ui, exception, msg):
  if isinstance(exception, error.Abort) and not exception.hint:
    exception.hint = msg
  else:
    ui.warn(..)
  raise exception

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -963,6 +963,11 @@ 
         if activebookmark and activebookmark in repo._bookmarks:
             bookmarks.activate(repo, activebookmark)
 
+    except Exception:
+        repo.ui.warn(_('warning: encountered an exception during rebase '
+            '--abort; the repository may not have been completely '
+            'cleaned up\n'))
+        raise
     finally:
         clearstatus(repo)
         repo.ui.warn(_('rebase aborted\n'))