Patchwork [4,of,4] templatekw: display non-current bookmarks (issue4552) (BC)

login
register
mail settings
Submitter Ryan McElroy
Date May 29, 2015, 5:22 p.m.
Message ID <95b095fb5d865ff5d357.1432920173@devbig105.prn2.facebook.com>
Download mbox | patch
Permalink /patch/9373/
State Accepted
Headers show

Comments

Ryan McElroy - May 29, 2015, 5:22 p.m.
# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1429114074 25200
#      Wed Apr 15 09:07:54 2015 -0700
# Node ID 95b095fb5d865ff5d357bd82905c21145fd4af65
# Parent  4dc7e32d957bcddb9dfe86c6d9443ba1c09fe1ac
templatekw: display non-current bookmarks (issue4552) (BC)

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.
Augie Fackler - May 29, 2015, 8 p.m.
On Fri, May 29, 2015 at 10:22:53AM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1429114074 25200
> #      Wed Apr 15 09:07:54 2015 -0700
> # Node ID 95b095fb5d865ff5d357bd82905c21145fd4af65
> # Parent  4dc7e32d957bcddb9dfe86c6d9443ba1c09fe1ac
> templatekw: display non-current bookmarks (issue4552) (BC)

I'm...not sure what this accomplishes? Can I get some sort of
before-and-after that demonstrates the BC easily?

>
> 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/README b/README
> --- a/README
> +++ b/README
> @@ -18,3 +18,4 @@ Running without installing:
>
>  See http://mercurial.selenic.com/ for detailed installation
>  instructions, platform-specific notes, and Mercurial user information.
> +

Your stray whitespace  change is confusing.

> diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
> --- a/mercurial/templatekw.py
> +++ b/mercurial/templatekw.py
> @@ -230,12 +230,9 @@ def showcurrentbookmark(**args):
>  def showactivebookmark(**args):
>      """:activetbookmark: String. The active bookmark, if it is
>      associated with the changeset"""
> -    import bookmarks as bookmarks # to avoid circular import issues
> -    repo = args['repo']
> -    if bookmarks.isactivewdirparent(repo):
> -        active = repo._activebookmark
> -        if active in args['ctx'].bookmarks():
> -            return active
> +    active = args['repo']._activebookmark
> +    if active and active in args['ctx'].bookmarks():
> +        return active
>      return ''
>
>  def showdate(repo, ctx, templ, **args):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - May 29, 2015, 8:05 p.m.
On 05/29/2015 10:22 AM, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1429114074 25200
> #      Wed Apr 15 09:07:54 2015 -0700
> # Node ID 95b095fb5d865ff5d357bd82905c21145fd4af65
> # Parent  4dc7e32d957bcddb9dfe86c6d9443ba1c09fe1ac
> templatekw: display non-current bookmarks (issue4552) (BC)
>
> 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.

 From what I understand, This let '{currentbookmark}' display something 
even when the 'active'/'current' bookmarks is not on "." (something that 
can happen after a pull).

If I get that right, this should go into the commit description.

(a test would also be great)

> diff --git a/README b/README

boo

Patch

diff --git a/README b/README
--- a/README
+++ b/README
@@ -18,3 +18,4 @@  Running without installing:
 
 See http://mercurial.selenic.com/ for detailed installation
 instructions, platform-specific notes, and Mercurial user information.
+
diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
--- a/mercurial/templatekw.py
+++ b/mercurial/templatekw.py
@@ -230,12 +230,9 @@  def showcurrentbookmark(**args):
 def showactivebookmark(**args):
     """:activetbookmark: String. The active bookmark, if it is
     associated with the changeset"""
-    import bookmarks as bookmarks # to avoid circular import issues
-    repo = args['repo']
-    if bookmarks.isactivewdirparent(repo):
-        active = repo._activebookmark
-        if active in args['ctx'].bookmarks():
-            return active
+    active = args['repo']._activebookmark
+    if active and active in args['ctx'].bookmarks():
+        return active
     return ''
 
 def showdate(repo, ctx, templ, **args):