[PATCH 04 of 10] rebase: improve error message for base revision

[Pierre-Yves David]

On Sun, Jan 12, 2014 at 05:08:02PM +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski at unity3d.com>
> # Date 1384730518 18000
> #      Sun Nov 17 18:21:58 2013 -0500
> # Node ID 55325a119cbeea17c2379bbe23f20d6226442a03
> # Parent  ef40ccf4914c597e32885227dde08abe40e41ad5
> rebase: improve error message for base revision
> 
> Before it just said 'nothing to rebase', now it also hints to the reason:

I like the idea of improving rebase error message but this one about base is confusing.
The argument from base is "changeset to compute a base from" not "base changeset".

So I would made the following changes:

> nothing to rebase - NODE is both base and destination

nothing to rebase - NODE is both source and destination

> nothing to rebase - base NODE is already an ancestor of destination NODE

cannot rebase - destination NODE is based on source NODE

> 'base revset is empty',

nothing to rebase - can't compute base from empty revset



Moreover reading this test ouput puzzled me:

>   $ hg rebase
>   nothing to rebase - e7ec4e813ba6 is both base and destination

What about dedicated detection and a more specific message.

nothing to rebase - already on BRANCH tipmost head
(do you want to use to use --dest or --base?)


I starting to thing --base could have been better named --branch (but then
people would think it is about named branch)

> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -234,9 +234,27 @@ def rebase(ui, repo, **opts):
>                  assert rebaseset
>              else:
>                  base = scmutil.revrange(repo, [basef or '.'])
> +                if not base:
> +                    ui.status(_('nothing to rebase - '
> +                                'base revset is empty\n'))
> +                    return 1
> +                if base == [dest.rev()]:
> +                    ui.status(_('nothing to rebase - '
> +                                '%s is both base and destination\n') % dest)
> +                    return 1
> +                if not repo.revs('%ld - ::%d', base, dest):
> +                    ui.status(_('nothing to rebase - base %s is already an '
> +                                'ancestor of destination %s\n') %
> +                              ('+'.join(str(repo[r]) for r in base),
> +                               dest))
> +                    return 1
>                  rebaseset = repo.revs(
>                      '(children(ancestor(%ld, %d)) and ::(%ld))::',
>                      base, dest, base)
> +                if not rebaseset: # cannot happen?
> +                    ui.status(_('nothing to rebase from %s to %s\n') %
> +                              ('+'.join(str(repo[r]) for r in base), dest))
> +                    return 1
>              if rebaseset:
>                  root = min(rebaseset)
>              else:
> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> --- a/tests/test-largefiles.t
> +++ b/tests/test-largefiles.t
> @@ -977,7 +977,7 @@ rebased or not.
>    M sub2/large6
>    saved backup bundle to $TESTTMP/d/.hg/strip-backup/f574fb32bb45-backup.hg (glob)
>    0 largefiles cached
> -  nothing to rebase
> +  nothing to rebase - 598410d3eb9a is both base and destination
>    $ [ -f .hg/largefiles/e166e74c7303192238d60af5a9c4ce9bef0b7928 ]
>    $ hg log --template '{rev}:{node|short}  {desc|firstline}\n'
>    9:598410d3eb9a  modify normal file largefile in repo d
> diff --git a/tests/test-rebase-parameters.t b/tests/test-rebase-parameters.t
> --- a/tests/test-rebase-parameters.t
> +++ b/tests/test-rebase-parameters.t
> @@ -80,13 +80,13 @@ These fail:
>    [255]
>  
>    $ hg rebase
> -  nothing to rebase
> +  nothing to rebase - e7ec4e813ba6 is both base and destination
>    [1]
>  
>    $ hg up -q 7
>  
>    $ hg rebase --traceback
> -  nothing to rebase
> +  nothing to rebase - base 02de42196ebe is already an ancestor of destination e7ec4e813ba6
>    [1]
>  
>    $ hg rebase -r '1 & !1'
> @@ -97,6 +97,9 @@ These fail:
>    nothing to rebase - source revset is empty
>    [1]
>  
> +  $ hg rebase --base '1 & !1'
> +  nothing to rebase - base revset is empty
> +  [1]
>  
>  These work:
>  
> diff --git a/tests/test-rebase-pull.t b/tests/test-rebase-pull.t
> --- a/tests/test-rebase-pull.t
> +++ b/tests/test-rebase-pull.t
> @@ -84,7 +84,7 @@ Invoke pull --rebase and nothing to reba
>    adding manifests
>    adding file changes
>    added 1 changesets with 1 changes to 1 files
> -  nothing to rebase
> +  nothing to rebase - base 783333faa078 is already an ancestor of destination 77ae9631bcca
>    1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    updating bookmark norebase
>  
> diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t
> --- a/tests/test-rebase-scenario-global.t
> +++ b/tests/test-rebase-scenario-global.t
> @@ -235,7 +235,7 @@ These will abort gracefully (using --bas
>  G onto G - rebase onto same changeset:
>  
>    $ hg rebase -b 6 -d 6
> -  nothing to rebase
> +  nothing to rebase - eea13746799a is both base and destination
>    [1]
>  
>  G onto F - rebase onto an ancestor:
> @@ -247,7 +247,7 @@ G onto F - rebase onto an ancestor:
>  F onto G - rebase onto a descendant:
>  
>    $ hg rebase -b 5 -d 6
> -  nothing to rebase
> +  nothing to rebase - base 24b6387c8c8c is already an ancestor of destination eea13746799a
>    [1]
>  
>  C onto A - rebase onto an ancestor:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20140113/977d2445/attachment.pgp>