[PATCH 1 of 3] bookmarks: simplify iscurrent to isactivecurrent (API)

[Ryan McElroy]

On 5/7/2015 2:56 PM, Martin von Zweigbergk wrote:
>
>
> On Thu, May 7, 2015 at 1:59 PM Ryan McElroy <rmcelroy at fb.com 
> <mailto:rmcelroy at fb.com>> wrote:
>
>     # HG changeset patch
>     # User Ryan McElroy <rmcelroy at fb.com <mailto:rmcelroy at fb.com>>
>     # Date 1429040715 25200
>     #      Tue Apr 14 12:45:15 2015 -0700
>     # Node ID ccb7b2b9a2f0dc5ac57e2e13e48180a03ad6d175
>     # Parent  17ba4ccd20b48511b3d06ab47fb1b2faf31410d7
>     bookmarks: simplify iscurrent to isactivecurrent (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.
>
>     diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
>     --- a/mercurial/bookmarks.py
>     +++ b/mercurial/bookmarks.py
>     @@ -165,17 +165,14 @@ def deactivate(repo):
>          finally:
>              wlock.release()
>
>     -def iscurrent(repo, mark=None, parents=None):
>     -    '''Tell whether the current bookmark is also active
>     -
>     -    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()]
>     +def isactivecurrent(repo):
>     +    """
>     +    Tell whether the 'active' bookmark is also 'current' -- eg,
>     points to
>     +    working directory parent(s). 
>
>
> In the description, you said that 'active' and 'current' are 
> uses interchangeably and now you're standardizing on 'active', but 
> here it seems like they now start to mean different things, or the 
> above docstring would be pointless ("whether the active bookmark is 
> also the active bookmark"). Assuming that you want them to mean 
> different things, could you describe the difference in the change 
> description?
>
> Also, I'm almost certain you meant "i.e.", not "e.g.".

You've caught me in a Latin mixup! Indeed, I should have used i.e.

This function alone is the epitome of what is wrong with the "active" vs 
"current" terminology.
"active" refers to up to one bookmark that will be updated when we make 
a new commit. Lots of people have it show up in their command prompt too.
"current" refers to whatever the current working directory's parents 
are, eg, the '.' commit (technically, p1() and p2())

The whole thing is messed up and this just clarifies the messiness. A 
future (BC) patch in this mega-series will eliminate this command 
altogether.
Regardless, I'll clean up the description and the latin and send a V2. 
Are the other two patches to your liking though?

>     +    """
>     +    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 updatecurrentbookmark(repo, oldnode, curbranch):
>     @@ -210,7 +207,7 @@ def calculateupdate(ui, repo, checkout):
>          movemarkfrom = None
>          if checkout is None:
>              curmark = repo._activebookmark
>     -        if iscurrent(repo):
>     +        if isactivecurrent(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.isactivecurrent(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.isactivecurrent(repo):
>              current = repo._activebookmark
>              if current in args['ctx'].bookmarks():
>                  return current
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel at selenic.com <mailto:Mercurial-devel at selenic.com>
>     http://selenic.com/mailman/listinfo/mercurial-devel
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20150507/2b456f24/attachment-0002.html>