Patchwork [03,of,10] rebase: improve error message for empty --source set

login
register
mail settings
Submitter Mads Kiilerich
Date Jan. 12, 2014, 4:08 p.m.
Message ID <ef40ccf4914c597e3288.1389542881@localhost.localdomain>
Download mbox | patch
Permalink /patch/3297/
State Superseded
Headers show

Comments

Mads Kiilerich - Jan. 12, 2014, 4:08 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1384730518 18000
#      Sun Nov 17 18:21:58 2013 -0500
# Node ID ef40ccf4914c597e32885227dde08abe40e41ad5
# Parent  d4c3a8ae89b0e2bf1e8a0bebb3fd6ba49ad1ac62
rebase: improve error message for empty --source set

Before it just said 'nothing to rebase', now it hints to the reason: '--source
revset is empty - nothing to rebase'.
Pierre-Yves David - Jan. 13, 2014, 7:10 a.m.
On Sun, Jan 12, 2014 at 05:08:01PM +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1384730518 18000
> #      Sun Nov 17 18:21:58 2013 -0500
> # Node ID ef40ccf4914c597e32885227dde08abe40e41ad5
> # Parent  d4c3a8ae89b0e2bf1e8a0bebb3fd6ba49ad1ac62
> rebase: improve error message for empty --source set

This series is pure gold^W^W^Wlooks good to me up to this revision (included).

I've some feedback on other changeset.

Note that using ui.status for error message sounds wrong, but the previous code
was doing it before.
Mads Kiilerich - Jan. 13, 2014, 1:43 p.m.
On 01/13/2014 08:10 AM, Pierre-Yves David wrote:
> On Sun, Jan 12, 2014 at 05:08:01PM +0100, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1384730518 18000
>> #      Sun Nov 17 18:21:58 2013 -0500
>> # Node ID ef40ccf4914c597e32885227dde08abe40e41ad5
>> # Parent  d4c3a8ae89b0e2bf1e8a0bebb3fd6ba49ad1ac62
>> rebase: improve error message for empty --source set
> This series is pure gold^W^W^Wlooks good to me up to this revision (included).
>
> I've some feedback on other changeset.
>
> Note that using ui.status for error message sounds wrong, but the previous code
> was doing it before.

I agree but didn't try to change the existing convention. Some of the 
failures should probably be aborts ... and the remaining ones should be 
"I did what you said, but note that it was a NOP" which is fine as 
ui.status. A NOP should probably end up as an abort for "hg rebase" but 
not for "hg pull --rebase".

/Mads
Pierre-Yves David - Jan. 13, 2014, 7:33 p.m.
On Mon, Jan 13, 2014 at 02:43:00PM +0100, Mads Kiilerich wrote:
> On 01/13/2014 08:10 AM, Pierre-Yves David wrote:
> >On Sun, Jan 12, 2014 at 05:08:01PM +0100, Mads Kiilerich wrote:
> >># HG changeset patch
> >># User Mads Kiilerich <madski@unity3d.com>
> >># Date 1384730518 18000
> >>#      Sun Nov 17 18:21:58 2013 -0500
> >># Node ID ef40ccf4914c597e32885227dde08abe40e41ad5
> >># Parent  d4c3a8ae89b0e2bf1e8a0bebb3fd6ba49ad1ac62
> >>rebase: improve error message for empty --source set
> >This series is pure gold^W^W^Wlooks good to me up to this revision (included).
> >
> >I've some feedback on other changeset.
> >
> >Note that using ui.status for error message sounds wrong, but the previous code
> >was doing it before.
> 
> I agree but didn't try to change the existing convention. Some of
> the failures should probably be aborts ... and the remaining ones
> should be "I did what you said, but note that it was a NOP" which is
> fine as ui.status. A NOP should probably end up as an abort for "hg
> rebase" but not for "hg pull --rebase".

I was just noticing it while reviewing that. I agree this is out of this series scope.
Augie Fackler - Jan. 15, 2014, 4:03 p.m.
On Mon, Jan 13, 2014 at 08:10:09AM +0100, Pierre-Yves David wrote:
> On Sun, Jan 12, 2014 at 05:08:01PM +0100, Mads Kiilerich wrote:
> > # HG changeset patch
> > # User Mads Kiilerich <madski@unity3d.com>
> > # Date 1384730518 18000
> > #      Sun Nov 17 18:21:58 2013 -0500
> > # Node ID ef40ccf4914c597e32885227dde08abe40e41ad5
> > # Parent  d4c3a8ae89b0e2bf1e8a0bebb3fd6ba49ad1ac62
> > rebase: improve error message for empty --source set
>
> This series is pure gold^W^W^Wlooks good to me up to this revision (included).

I'm going to queue the first three in this series, as I think despite
some awkwardness they are a strict improvement. I'm not looking at the
rest of the series at all.

>
> I've some feedback on other changeset.
>
> Note that using ui.status for error message sounds wrong, but the previous code
> was doing it before.
>
> --
> Pierre-Yves David



> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -226,7 +226,12 @@  def rebase(ui, repo, **opts):
                     return 1
             elif srcf:
                 src = scmutil.revrange(repo, [srcf])
+                if not src:
+                    ui.status(_('nothing to rebase - '
+                                'source revset is empty\n'))
+                    return 1
                 rebaseset = repo.revs('(%ld)::', src)
+                assert rebaseset
             else:
                 base = scmutil.revrange(repo, [basef or '.'])
                 rebaseset = repo.revs(
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
@@ -93,6 +93,10 @@  These fail:
   nothing to rebase - rev revset is empty
   [1]
 
+  $ hg rebase --source '1 & !1'
+  nothing to rebase - source revset is empty
+  [1]
+
 
 These work: