[PATCH] rebase: restore active bookmark after rebase --continue

[Kevin Bullock]

On 11 Mar 2013, at 6:21 PM, Durham Goode wrote:

> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1363041448 25200
> #      Mon Mar 11 15:37:28 2013 -0700
> # Node ID 344cc2d5e092186bc6d593b53f4c1a338abc0af7
> # Parent  2b1729b20820c0eeb0857bb224d009db698faeef
> rebase: restore active bookmark after rebase --continue
> 
> When a rebase has conflicts and the user uses rebase --continue, the previously
> active bookmark was not being made active once again. With this change that
> bookmark is made active again, just as if the rebase had never been interrupted.

Sounds like a bug. This probably ought to be flagged for stable.

> This changes the rebasestate file format, but since that file is transient, it
> should be extremely rare for it to cause a problem (the user would have to
> upgrade mercurial in the middle of an interrupted rebase).
> 
> Adds a test to verify the new behavior. I manually tested continuing rebases
> with and without an active bookmark, and with and without being on the bookmark
> being rebased.
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -112,6 +112,7 @@
>     Returns 0 on success, 1 if nothing to rebase.
>     """
>     originalwd = target = None
> +    activebookmark = None
>     external = nullrev
>     state = {}
>     skipped = set()
> @@ -159,7 +160,7 @@
>                 ui.warn(_('tool option will be ignored\n'))
> 
>             (originalwd, target, state, skipped, collapsef, keepf,
> -                                keepbranchesf, external) = restorestatus(repo)
> +                keepbranchesf, external, activebookmark) = restorestatus(repo)

Huh, check-code and pyflakes are both stunningly silent about these whitespace changes. (I consider what you did to be -correcting- the whitespace, just surprised we don't have a rule for these.)

> [...]
> @@ -560,6 +563,8 @@
>                 keep = bool(int(l))
>             elif i == 5:
>                 keepbranches = bool(int(l))
> +            elif i == 6:
> +                activebookmark = l

Seems like you could guard against a stale state from an old version by saying `elif i == 6 and ':' not in l:` here.

> diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
> --- a/tests/test-rebase-conflicts.t
> +++ b/tests/test-rebase-conflicts.t
> [...]
> +  $ hg bookmarks
> +   * mybook                    5:d67b21408fc0
> +

An explanatory comment would be good here ("bookmark stays active after rebase --continue" or somesuch).

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock