[PATCH] mq: informative error instead of stack trace when qpush-ing to an earlier patch (issue2587)

[Afuna]

# HG changeset patch
# User Afuna
# Date 1297425323 -28800
# Node ID 550dcecf238306def4419d816c9d9930e54bf9eb
# Parent  d4ab9486e514dd24e21a2ca3b6c439ea13d85cab
mq: informative error instead of stack trace when qpush-ing to an earlier patch (issue2587)

I made this check come before the check as to whether the series was fully applied (seemed to make more sense that way). Have added a test for this case, because before the patch was applied, the behavior differed depending on whether you were at the top of the queue or not ("fully applied" for the former; an ugly stack trace for the latter), but I'm not sure we need it now.


Since we know that the patch exists, and we know that the user is trying to go to that patch, it might be possible for us to just do the equivalent of qpop patch instead of telling the user to do so. On the other hand -- there might be some confusion if the user expected the previously guarded patch to be on top of whatever patch they're currently, so popping their current patch could be unexpected behavior.

I've gone with just printing out the warning / instructions.


And finally one question, what should this return? I see above a "all patches are currently applied" which returns 0; another "patch series already fully applied" which returns 1, and a bunch of util.Abort calls for other errors. I can't quite tell where this falls, so the current return value is just a guess.

diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -1088,6 +1088,10 @@ class queue(object):
             # apply). This allows a loop of "...while hg qpush..." to
             # work as it detects an error when done
             start = self.series_end()
+            if patch and self.series.index(patch) < start:
+                self.ui.warn(_("patch is earlier in the series. Try hg qpop '%s'\n"
+                    % patch))
+                return 1
             if start == len(self.series):
                 self.ui.warn(_('patch series already fully applied\n'))
                 return 1
diff --git a/tests/test-mq-qpush-fail.t b/tests/test-mq-qpush-fail.t
--- a/tests/test-mq-qpush-fail.t
+++ b/tests/test-mq-qpush-fail.t
@@ -88,3 +88,49 @@ qpush should fail the same way as below
   applying patch1
   unable to read patch1
   [1]
+
+Test qpush to a patch below the currently applied patch.
+
+  $ hg qq -c guardedseriesorder
+  $ hg qnew a
+  $ hg qguard +block
+  $ hg qnew b
+  $ hg qnew c
+
+  $ hg qpop -a
+  popping c
+  popping b
+  popping a
+  patch queue now empty
+
+try to push and pop while a is guarded
+
+  $ hg qpush a
+  cannot push 'a' - guarded by ['+block']
+  [1]
+  $ hg qpush -a
+  applying b
+  patch b is empty
+  applying c
+  patch c is empty
+  now at: c
+
+now try it when a is unguarded, and we're at the top of the queue
+  $ hg qsel block
+  number of guarded, applied patches has changed from 1 to 0
+  $ hg qpush b
+  abort: cannot push to a previous patch: b
+  [255]
+  $ hg qpush a
+  patch is earlier in the series. Try hg qpop 'a'
+  [1]
+
+and now we try it one more time with a unguarded, while we're not at the top of the queue
+
+  $ hg qpop b
+  popping c
+  now at: b
+  $ hg qpush a
+  patch is earlier in the series. Try hg qpop 'a'
+  [1]
+