Patchwork [evolve-ext] evolve: improve error message if unstable changes are disallowed

login
register
mail settings
Submitter Pulkit Goyal
Date Nov. 23, 2016, 3:36 p.m.
Message ID <32083f1f0c67341e5b4c.1479915389@pulkit-goyal>
Download mbox | patch
Permalink /patch/17716/
State Changes Requested
Headers show

Comments

Pulkit Goyal - Nov. 23, 2016, 3:36 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1479915042 -19800
#      Wed Nov 23 21:00:42 2016 +0530
# Node ID 32083f1f0c67341e5b4c22e880b70ccc4e0fc088
# Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
evolve: improve error message if unstable changes are disallowed

I saw a question on stackoverflow why evolve reports something like cannot
fold chain not ending with head. Even I was confused the first time about the
behavior. The error message can be improved to avoid confusion to people who
are unaware about the config in future.
Mateusz Kwapich - Nov. 23, 2016, 4:19 p.m.
Excerpts from Pulkit Goyal's message of 2016-11-23 21:06:29 +0530:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1479915042 -19800
> #      Wed Nov 23 21:00:42 2016 +0530
> # Node ID 32083f1f0c67341e5b4c22e880b70ccc4e0fc088
> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
> evolve: improve error message if unstable changes are disallowed
> 
> I saw a question on stackoverflow why evolve reports something like cannot
> fold chain not ending with head. Even I was confused the first time about the
> behavior. The error message can be improved to avoid confusion to people who
> are unaware about the config in future.

That sounds like a very useful information. It sucks that the errors
have newlines in them. I think we can avoid it - see my inline comments.

> 
> diff -r cb2bac3253fb -r 32083f1f0c67 hgext/evolve.py
> --- a/hgext/evolve.py    Wed Nov 02 18:56:44 2016 +0100
> +++ b/hgext/evolve.py    Wed Nov 23 21:00:42 2016 +0530
> @@ -2514,7 +2514,8 @@
>              raise error.Abort('nothing to prune')
>  
>          if _disallowednewunstable(repo, revs):
> -            raise error.Abort(_("cannot prune in the middle of a stack"))
> +            raise error.Abort(_("cannot prune in the middle of a stack\n"\
> +                                "new unstable changesets are not allowed"))
This error doesn't have hint - maybe we could use it for that purpose.
>  
>          # defines successors changesets
>          sucs = scmutil.revrange(repo, succs)
> @@ -3234,7 +3235,8 @@
>              newunstable = _disallowednewunstable(repo, revs)
>              if newunstable:
>                  raise error.Abort(
> -                    _('cannot edit commit information in the middle of a stack'),
> +                    _('cannot edit commit information in the middle of a stack\n'\
> +                        'new unstable changesets are not allowed'),
>                      hint=_('%s will be affected') % repo[newunstable.first()])
I would change the hint to say something along:
'%s would become unstable but new unstable commits are not allowed'
>              root = head = repo[revs.first()]
>  
> @@ -3299,7 +3301,8 @@
>      head = repo[heads.first()]
>      if _disallowednewunstable(repo, revs):
>          raise error.Abort(_("cannot fold chain not ending with a head "\
> -                            "or with branching"))
> +                            "or with branching\nnew unstable changesets are"\
> +                            " not allowed"))
I would move this to hint param.
>      return root, head
>  
>  def _disallowednewunstable(repo, revs):
> diff -r cb2bac3253fb -r 32083f1f0c67 tests/test-evolve.t
> --- a/tests/test-evolve.t    Wed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-evolve.t    Wed Nov 23 21:00:42 2016 +0530
> @@ -1301,9 +1301,11 @@
>    created new head
>    $ hg prune '26 + 27'
>    abort: cannot prune in the middle of a stack
> +  new unstable changesets are not allowed
>    [255]
>    $ hg prune '19::28'
>    abort: cannot prune in the middle of a stack
> +  new unstable changesets are not allowed
>    [255]
>    $ hg prune '26::'
>    3 changesets pruned
> @@ -1338,9 +1340,11 @@
>  
>    $ hg fold --exact "19 + 18"
>    abort: cannot fold chain not ending with a head or with branching
> +  new unstable changesets are not allowed
>    [255]
>    $ hg fold --exact "18::29"
>    abort: cannot fold chain not ending with a head or with branching
> +  new unstable changesets are not allowed
>    [255]
>    $ hg fold --exact "19::"
>    2 changesets folded
> @@ -1483,10 +1487,12 @@
>  check that metaedit respects allowunstable
>    $ hg metaedit '.^' --config 'experimental.evolution=createmarkers, allnewcommands'
>    abort: cannot edit commit information in the middle of a stack
> +  new unstable changesets are not allowed
>    (c904da5245b0 will be affected)
>    [255]
>    $ hg metaedit '18::20' --fold --config 'experimental.evolution=createmarkers, allnewcommands'
>    abort: cannot fold chain not ending with a head or with branching
> +  new unstable changesets are not allowed
>    [255]
>    $ hg metaedit --user foobar
>    0 files updated, 0 files merged, 0 files removed, 0 files unresolved
timeless - Nov. 24, 2016, 2:04 a.m.
Mateusz Kwapich wrote:
> Pulkit wrote:
>> +            raise error.Abort(_("cannot prune in the middle of a stack\n"\
>> +                                "new unstable changesets are not allowed"))
> This error doesn't have hint - maybe we could use it for that purpose.

I should spend some time working on these messages...

>> +                    _('cannot edit commit information in the middle of a stack\n'\
>> +                        'new unstable changesets are not allowed'),
>>                      hint=_('%s will be affected') % repo[newunstable.first()])
> I would change the hint to say something along:
> '%s would become unstable but new unstable commits are not allowed'

I'm tempted to say "and" instead of "but"...

>> @@ -3299,7 +3301,8 @@
>>      head = repo[heads.first()]
>>      if _disallowednewunstable(repo, revs):
>>          raise error.Abort(_("cannot fold chain not ending with a head "\

"chain not ending with a head" needs work :/

>> +                            "or with branching\nnew unstable changesets are"\
>> +                            " not allowed"))
> I would move this to hint param.

I agree with Mateusz's suggestions in general.
Pierre-Yves David - Nov. 24, 2016, 5:31 p.m.
On 11/23/2016 05:19 PM, Mateusz Kwapich wrote:
> Excerpts from Pulkit Goyal's message of 2016-11-23 21:06:29 +0530:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1479915042 -19800
>> #      Wed Nov 23 21:00:42 2016 +0530
>> # Node ID 32083f1f0c67341e5b4c22e880b70ccc4e0fc088
>> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>> evolve: improve error message if unstable changes are disallowed
>>
>> I saw a question on stackoverflow why evolve reports something like cannot
>> fold chain not ending with head. Even I was confused the first time about the
>> behavior. The error message can be improved to avoid confusion to people who
>> are unaware about the config in future.
>
> That sounds like a very useful information. It sucks that the errors
> have newlines in them. I think we can avoid it - see my inline comments.

Yet, improving that message is a good idea. As mateuz pointed, we avoid 
new line in error message:

   https://www.mercurial-scm.org/wiki/UIGuideline#error_message

In this case pointing to some help about the config would be a good idea 
(this might involve creating such help o:-) )

Thanks for finding it and working on this.
Pulkit Goyal - Nov. 24, 2016, 5:48 p.m.
On Thu, Nov 24, 2016 at 11:01 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 11/23/2016 05:19 PM, Mateusz Kwapich wrote:
>>
>> Excerpts from Pulkit Goyal's message of 2016-11-23 21:06:29 +0530:
>>>
>>> # HG changeset patch
>>> # User Pulkit Goyal <7895pulkit@gmail.com>
>>> # Date 1479915042 -19800
>>> #      Wed Nov 23 21:00:42 2016 +0530
>>> # Node ID 32083f1f0c67341e5b4c22e880b70ccc4e0fc088
>>> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
>>> evolve: improve error message if unstable changes are disallowed
>>>
>>> I saw a question on stackoverflow why evolve reports something like
>>> cannot
>>> fold chain not ending with head. Even I was confused the first time about
>>> the
>>> behavior. The error message can be improved to avoid confusion to people
>>> who
>>> are unaware about the config in future.
>>
>>
>> That sounds like a very useful information. It sucks that the errors
>> have newlines in them. I think we can avoid it - see my inline comments.
>
>
> Yet, improving that message is a good idea. As mateuz pointed, we avoid new
> line in error message:
>
>   https://www.mercurial-scm.org/wiki/UIGuideline#error_message
>
> In this case pointing to some help about the config would be a good idea
> (this might involve creating such help o:-) )

Well I send a V2 incorporating Mateusz feedback. I also thought of
referring to experimental.evolution but I couldn't find a good way, or
error message. If config thing is better, drop the V2, I will find a
way to refer to config thing. :)

> Thanks for finding it and working on this.
>
> --
> Pierre-Yves David

Patch

diff -r cb2bac3253fb -r 32083f1f0c67 hgext/evolve.py
--- a/hgext/evolve.py	Wed Nov 02 18:56:44 2016 +0100
+++ b/hgext/evolve.py	Wed Nov 23 21:00:42 2016 +0530
@@ -2514,7 +2514,8 @@ 
             raise error.Abort('nothing to prune')
 
         if _disallowednewunstable(repo, revs):
-            raise error.Abort(_("cannot prune in the middle of a stack"))
+            raise error.Abort(_("cannot prune in the middle of a stack\n"\
+                                "new unstable changesets are not allowed"))
 
         # defines successors changesets
         sucs = scmutil.revrange(repo, succs)
@@ -3234,7 +3235,8 @@ 
             newunstable = _disallowednewunstable(repo, revs)
             if newunstable:
                 raise error.Abort(
-                    _('cannot edit commit information in the middle of a stack'),
+                    _('cannot edit commit information in the middle of a stack\n'\
+                        'new unstable changesets are not allowed'),
                     hint=_('%s will be affected') % repo[newunstable.first()])
             root = head = repo[revs.first()]
 
@@ -3299,7 +3301,8 @@ 
     head = repo[heads.first()]
     if _disallowednewunstable(repo, revs):
         raise error.Abort(_("cannot fold chain not ending with a head "\
-                            "or with branching"))
+                            "or with branching\nnew unstable changesets are"\
+                            " not allowed"))
     return root, head
 
 def _disallowednewunstable(repo, revs):
diff -r cb2bac3253fb -r 32083f1f0c67 tests/test-evolve.t
--- a/tests/test-evolve.t	Wed Nov 02 18:56:44 2016 +0100
+++ b/tests/test-evolve.t	Wed Nov 23 21:00:42 2016 +0530
@@ -1301,9 +1301,11 @@ 
   created new head
   $ hg prune '26 + 27'
   abort: cannot prune in the middle of a stack
+  new unstable changesets are not allowed
   [255]
   $ hg prune '19::28'
   abort: cannot prune in the middle of a stack
+  new unstable changesets are not allowed
   [255]
   $ hg prune '26::'
   3 changesets pruned
@@ -1338,9 +1340,11 @@ 
 
   $ hg fold --exact "19 + 18"
   abort: cannot fold chain not ending with a head or with branching
+  new unstable changesets are not allowed
   [255]
   $ hg fold --exact "18::29"
   abort: cannot fold chain not ending with a head or with branching
+  new unstable changesets are not allowed
   [255]
   $ hg fold --exact "19::"
   2 changesets folded
@@ -1483,10 +1487,12 @@ 
 check that metaedit respects allowunstable
   $ hg metaedit '.^' --config 'experimental.evolution=createmarkers, allnewcommands'
   abort: cannot edit commit information in the middle of a stack
+  new unstable changesets are not allowed
   (c904da5245b0 will be affected)
   [255]
   $ hg metaedit '18::20' --fold --config 'experimental.evolution=createmarkers, allnewcommands'
   abort: cannot fold chain not ending with a head or with branching
+  new unstable changesets are not allowed
   [255]
   $ hg metaedit --user foobar
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved