[PATCH V2] bookmarks: simplify iscurrent to isactivewdirparent (API)

[Augie Fackler]

On Thu, May 07, 2015 at 03:52:56PM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy at fb.com>
> # Date 1429040715 25200
> #      Tue Apr 14 12:45:15 2015 -0700
> # Node ID 1c885f2c5086d419c8a082ff7204fbce27c6c395
> # Parent  10f712a17574852069d60db0567b2246a389d13f
> bookmarks: simplify iscurrent to isactivewdirparent (API)

Queued, thanks.

Something about the docstring on isactivewdirparent feels unclear, but
I can't put my finger on it right now, and this is a strict
improvement over the past, so I'm just queueing it in the name of
not letting the perfect be the enemy of teh good.

>
> Previously this function accepted two optional parameters that were unused by
> any callers and complicated the function.
>
> Today, the terms 'active' and 'current' are interchangeably used throughout the
> codebase in reference to the active bookmark (the bookmark that will be updated
> with the next commit). This leads to confusion among developers and users.
> This patch is part of a series to standardize the usage to 'active' throughout
> the mercurial codebase and user interface.
>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -165,17 +165,18 @@ def deactivate(repo):
>      finally:
>          wlock.release()
>
> -def iscurrent(repo, mark=None, parents=None):
> -    '''Tell whether the current bookmark is also active
> +def isactivewdirparent(repo):
> +    """
> +    Tell whether the 'active' bookmark (the one that follows new commits)
> +    points to one of the parents of the current working directory (wdir).
>
> -    I.e., the bookmark listed in .hg/bookmarks.current also points to a
> -    parent of the working directory.
> -    '''
> -    if not mark:
> -        mark = repo._activebookmark
> -    if not parents:
> -        parents = [p.node() for p in repo[None].parents()]
> +    While this is normally the case, it can on occasion be false; for example,
> +    immediately after a pull, the active bookmark can be moved to point
> +    to a place different than the wdir. This is solved by running `hg update`.
> +    """
> +    mark = repo._activebookmark
>      marks = repo._bookmarks
> +    parents = [p.node() for p in repo[None].parents()]
>      return (mark in marks and marks[mark] in parents)
>
>  def deletedivergent(repo, deletefrom, bm):
> @@ -201,7 +202,7 @@ def calculateupdate(ui, repo, checkout):
>      movemarkfrom = None
>      if checkout is None:
>          curmark = repo._activebookmark
> -        if iscurrent(repo):
> +        if isactivewdirparent(repo):
>              movemarkfrom = repo['.'].node()
>          elif curmark:
>              ui.status(_("updating to active bookmark %s\n") % curmark)
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2721,7 +2721,7 @@ def buildcommittext(repo, ctx, subs, ext
>          edittext.append(_("HG: branch merge"))
>      if ctx.branch():
>          edittext.append(_("HG: branch '%s'") % ctx.branch())
> -    if bookmarks.iscurrent(repo):
> +    if bookmarks.isactivewdirparent(repo):
>          edittext.append(_("HG: bookmark '%s'") % repo._activebookmark)
>      edittext.extend([_("HG: subrepo %s") % s for s in subs])
>      edittext.extend([_("HG: added %s") % f for f in added])
> diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
> --- a/mercurial/templatekw.py
> +++ b/mercurial/templatekw.py
> @@ -226,7 +226,7 @@ def showcurrentbookmark(**args):
>      associated with the changeset"""
>      import bookmarks as bookmarks # to avoid circular import issues
>      repo = args['repo']
> -    if bookmarks.iscurrent(repo):
> +    if bookmarks.isactivewdirparent(repo):
>          current = repo._activebookmark
>          if current in args['ctx'].bookmarks():
>              return current
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel