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

# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1429040715 25200
#      Tue Apr 14 12:45:15 2015 -0700
# Node ID 1c885f2c5086d419c8a082ff7204fbce27c6c395
# Parent  10f712a17574852069d60db0567b2246a389d13f
bookmarks: simplify iscurrent to isactivewdirparent (API)

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.

On Thu, May 07, 2015 at 03:52:56PM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@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@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel