Patchwork [6,of,6,V4] commands: use bookmarks.validdest instead of duplicating logic

login
register
mail settings
Submitter Sean Farley
Date Jan. 17, 2014, 8:14 p.m.
Message ID <90bb5b7793efbb3f0952.1389989672@laptop.local>
Download mbox | patch
Permalink /patch/3379/
State Accepted
Commit 2cfb720592feb329eb5597100c3fc5dd48bafb15
Headers show

Comments

Sean Farley - Jan. 17, 2014, 8:14 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1389830113 21600
#      Wed Jan 15 17:55:13 2014 -0600
# Node ID 90bb5b7793efbb3f09529971079d36bd2c5cad5d
# Parent  2db9cc45ec27273af8ee1240390d051c585034d8
commands: use bookmarks.validdest instead of duplicating logic

Now that bookmarks.py has grown a validdest method that even handles successor
changesets, we use that instead of duplicating the logic in commands.py
Pierre-Yves David - Jan. 17, 2014, 8:53 p.m.
V4 looks good to me.

Thanks.

On 01/17/2014 12:14 PM, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1389830113 21600
> #      Wed Jan 15 17:55:13 2014 -0600
> # Node ID 90bb5b7793efbb3f09529971079d36bd2c5cad5d
> # Parent  2db9cc45ec27273af8ee1240390d051c585034d8
> commands: use bookmarks.validdest instead of duplicating logic
>
> Now that bookmarks.py has grown a validdest method that even handles successor
> changesets, we use that instead of duplicating the logic in commands.py
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -845,16 +845,14 @@ def bookmark(ui, repo, *names, **opts):
>                   # that contains a divergent bookmark
>                   if bmctx.rev() not in anc and target in divs:
>                       bookmarks.deletedivergent(repo, [target], mark)
>                       return
>   
> -                # consider successor changesets as well
> -                foreground = obsolete.foreground(repo, [marks[mark]])
>                   deletefrom = [b for b in divs
>                                 if repo[b].rev() in anc or b == target]
>                   bookmarks.deletedivergent(repo, deletefrom, mark)
> -                if bmctx.rev() in anc or target in foreground:
> +                if bookmarks.validdest(repo, bmctx, repo[target]):
>                       ui.status(_("moving bookmark '%s' forward from %s\n") %
>                                 (mark, short(bmctx.node())))
>                       return
>               raise util.Abort(_("bookmark '%s' already exists "
>                                  "(use -f to force)") % mark)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Jan. 17, 2014, 9:16 p.m.
On Fri, 2014-01-17 at 14:14 -0600, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1389830113 21600
> #      Wed Jan 15 17:55:13 2014 -0600
> # Node ID 90bb5b7793efbb3f09529971079d36bd2c5cad5d
> # Parent  2db9cc45ec27273af8ee1240390d051c585034d8
> commands: use bookmarks.validdest instead of duplicating logic

These are queued, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -845,16 +845,14 @@  def bookmark(ui, repo, *names, **opts):
                 # that contains a divergent bookmark
                 if bmctx.rev() not in anc and target in divs:
                     bookmarks.deletedivergent(repo, [target], mark)
                     return
 
-                # consider successor changesets as well
-                foreground = obsolete.foreground(repo, [marks[mark]])
                 deletefrom = [b for b in divs
                               if repo[b].rev() in anc or b == target]
                 bookmarks.deletedivergent(repo, deletefrom, mark)
-                if bmctx.rev() in anc or target in foreground:
+                if bookmarks.validdest(repo, bmctx, repo[target]):
                     ui.status(_("moving bookmark '%s' forward from %s\n") %
                               (mark, short(bmctx.node())))
                     return
             raise util.Abort(_("bookmark '%s' already exists "
                                "(use -f to force)") % mark)