Patchwork [1,of,2] histedit: limit mentioning histedit-last-edit.txt

login
register
mail settings
Submitter timeless@mozdev.org
Date Dec. 11, 2015, 7:22 a.m.
Message ID <88fc68941a015579f2f7.1449818546@waste.org>
Download mbox | patch
Permalink /patch/11972/
State Superseded
Commit ff0e4c8e642d58896f4edd9457b83c9ab8c0f680
Headers show

Comments

timeless@mozdev.org - Dec. 11, 2015, 7:22 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1449817689 0
#      Fri Dec 11 07:08:09 2015 +0000
# Node ID 88fc68941a015579f2f7b3132f8d21e13b4cbf56
# Parent  42aa0e570eaa364a622bc4443b0bcb79b1100a58
histedit: limit mentioning histedit-last-edit.txt

Before histedit-last-edit.txt would be mentioned for any failure.
After, it should only be mentioned for failures relating to user
input.
Pierre-Yves David - Dec. 11, 2015, 11:54 a.m.
On 12/11/2015 07:22 AM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1449817689 0
> #      Fri Dec 11 07:08:09 2015 +0000
> # Node ID 88fc68941a015579f2f7b3132f8d21e13b4cbf56
> # Parent  42aa0e570eaa364a622bc4443b0bcb79b1100a58
> histedit: limit mentioning histedit-last-edit.txt
>
> Before histedit-last-edit.txt would be mentioned for any failure.
> After, it should only be mentioned for failures relating to user
> input.

I'm not sure about this, there is still value to save (and point at) the 
user rules in failure -later- in the process.

(I agree that pointing at previously saved rules before user even get 
prompted for them seems silly)

>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -879,11 +879,6 @@
>           state.wlock = repo.wlock()
>           state.lock = repo.lock()
>           _histedit(ui, repo, state, *freeargs, **opts)
> -    except error.Abort:
> -        if repo.vfs.exists('histedit-last-edit.txt'):
> -            ui.warn(_('warning: histedit rules saved '
> -                      'to: .hg/histedit-last-edit.txt\n'))
> -        raise
>       finally:
>           release(state.lock, state.wlock)
>
> @@ -964,7 +959,7 @@
>           actions = parserules(rules, state)
>           ctxs = [repo[act.nodetoverify()] \
>                   for act in state.actions if act.nodetoverify()]
> -        verifyactions(actions, state, ctxs)
> +        warnverifyactions(ui, repo, actions, state, ctxs)
>           state.actions = actions
>           state.write()
>           return
> @@ -1047,7 +1042,7 @@
>               rules = f.read()
>               f.close()
>           actions = parserules(rules, state)
> -        verifyactions(actions, state, ctxs)
> +        warnverifyactions(ui, repo, actions, state, ctxs)
>
>           parentctxnode = repo[root].parents()[0].node()
>
> @@ -1193,6 +1188,15 @@
>           actions.append(action)
>       return actions
>
> +def warnverifyactions(ui, repo, actions, state, ctxs):
> +    try:
> +        verifyactions(actions, state, ctxs)
> +    except error.Abort:
> +        if repo.vfs.exists('histedit-last-edit.txt'):
> +            ui.warn(_('warning: histedit rules saved '
> +                      'to: .hg/histedit-last-edit.txt\n'))
> +        raise
> +
>   def verifyactions(actions, state, ctxs):
>       """Verify that there exists exactly one action per given changeset and
>       other constraints.
> diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
> --- a/tests/test-histedit-edit.t
> +++ b/tests/test-histedit-edit.t
> @@ -460,7 +460,6 @@
>     1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>     reverting a
>     1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> -  warning: histedit rules saved to: .hg/histedit-last-edit.txt
>     abort: cannot fold into public change 18aa70c8ad22
>     [255]
>     $ cat .hg/histedit-last-edit.txt

This test change seems to to be wrong. The user selected some action and 
this action selection is buggy. We should keep  the actions and point 
the user at them to give him a chance to fix them.
timeless - Dec. 28, 2015, 11:30 p.m.
Pierre-Yves David wrote:
> This test change seems to to be wrong. The user selected some action and
> this action selection is buggy. We should keep  the actions and point the
> user at them to give him a chance to fix them.

Indeed.
I've addressed that with:
histedit: check fold of public change during verify

as a bonus, the todo for that goes away :)

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -879,11 +879,6 @@ 
         state.wlock = repo.wlock()
         state.lock = repo.lock()
         _histedit(ui, repo, state, *freeargs, **opts)
-    except error.Abort:
-        if repo.vfs.exists('histedit-last-edit.txt'):
-            ui.warn(_('warning: histedit rules saved '
-                      'to: .hg/histedit-last-edit.txt\n'))
-        raise
     finally:
         release(state.lock, state.wlock)
 
@@ -964,7 +959,7 @@ 
         actions = parserules(rules, state)
         ctxs = [repo[act.nodetoverify()] \
                 for act in state.actions if act.nodetoverify()]
-        verifyactions(actions, state, ctxs)
+        warnverifyactions(ui, repo, actions, state, ctxs)
         state.actions = actions
         state.write()
         return
@@ -1047,7 +1042,7 @@ 
             rules = f.read()
             f.close()
         actions = parserules(rules, state)
-        verifyactions(actions, state, ctxs)
+        warnverifyactions(ui, repo, actions, state, ctxs)
 
         parentctxnode = repo[root].parents()[0].node()
 
@@ -1193,6 +1188,15 @@ 
         actions.append(action)
     return actions
 
+def warnverifyactions(ui, repo, actions, state, ctxs):
+    try:
+        verifyactions(actions, state, ctxs)
+    except error.Abort:
+        if repo.vfs.exists('histedit-last-edit.txt'):
+            ui.warn(_('warning: histedit rules saved '
+                      'to: .hg/histedit-last-edit.txt\n'))
+        raise
+
 def verifyactions(actions, state, ctxs):
     """Verify that there exists exactly one action per given changeset and
     other constraints.
diff --git a/tests/test-histedit-edit.t b/tests/test-histedit-edit.t
--- a/tests/test-histedit-edit.t
+++ b/tests/test-histedit-edit.t
@@ -460,7 +460,6 @@ 
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   reverting a
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
-  warning: histedit rules saved to: .hg/histedit-last-edit.txt
   abort: cannot fold into public change 18aa70c8ad22
   [255]
   $ cat .hg/histedit-last-edit.txt