Patchwork [7,of,7,bm-refactor,V3] bookmarks: factor method _printer out of for loop in printbookmarks

login
register
mail settings
Submitter Sean Farley
Date June 23, 2017, 6:33 p.m.
Message ID <1e90be070c35665fb3d7.1498242803@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/21637/
State Accepted
Headers show

Comments

Sean Farley - June 23, 2017, 6:33 p.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1498004300 25200
#      Tue Jun 20 17:18:20 2017 -0700
# Branch bm-refactor
# Node ID 1e90be070c35665fb3d75657563349650cc523a1
# Parent  5caa40d7fa1be3ff13a2e80a91234809cff5307a
bookmarks: factor method _printer out of for loop in printbookmarks

This allows even further customization via extensions for printing
bookmarks. For example, in hg-git this would allow printing remote refs
by just modifying the 'bmarks' parameter instead of reimplementing the
old commands.bookmarks method.

Furthermore, there is another benefit: now multiple extensions can
chain their custom data to bookmark printing. Previously, an extension
could have conflicting bookmark output due to which loaded first and
overrode commands.bookmarks. Now they can all play nicely together.
via Mercurial-devel - June 23, 2017, 6:55 p.m.
On Fri, Jun 23, 2017 at 11:33 AM, Sean Farley <sean@farley.io> wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1498004300 25200
> #      Tue Jun 20 17:18:20 2017 -0700
> # Branch bm-refactor
> # Node ID 1e90be070c35665fb3d75657563349650cc523a1
> # Parent  5caa40d7fa1be3ff13a2e80a91234809cff5307a
> bookmarks: factor method _printer out of for loop in printbookmarks
>
> This allows even further customization via extensions for printing
> bookmarks. For example, in hg-git this would allow printing remote refs
> by just modifying the 'bmarks' parameter instead of reimplementing the
> old commands.bookmarks method.
>
> Furthermore, there is another benefit: now multiple extensions can
> chain their custom data to bookmark printing. Previously, an extension
> could have conflicting bookmark output due to which loaded first and
> overrode commands.bookmarks. Now they can all play nicely together.
>
> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> index 518b362..797e3c1 100644
> --- a/mercurial/bookmarks.py
> +++ b/mercurial/bookmarks.py
> @@ -765,32 +765,43 @@ def addbookmarks(repo, tr, names, rev=No
>          activate(repo, newact)
>      elif cur != tgt and newact == repo._activebookmark:
>          deactivate(repo)
>      marks.recordchange(tr)
>
> +def _printer(ui, repo, bmarks, **opts):

nit: This actually prints the bookmarks. The name made me think it
would return an object that could be used for printing.
Sean Farley - June 23, 2017, 7:05 p.m.
Martin von Zweigbergk <martinvonz@google.com> writes:

> On Fri, Jun 23, 2017 at 11:33 AM, Sean Farley <sean@farley.io> wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1498004300 25200
>> #      Tue Jun 20 17:18:20 2017 -0700
>> # Branch bm-refactor
>> # Node ID 1e90be070c35665fb3d75657563349650cc523a1
>> # Parent  5caa40d7fa1be3ff13a2e80a91234809cff5307a
>> bookmarks: factor method _printer out of for loop in printbookmarks
>>
>> This allows even further customization via extensions for printing
>> bookmarks. For example, in hg-git this would allow printing remote refs
>> by just modifying the 'bmarks' parameter instead of reimplementing the
>> old commands.bookmarks method.
>>
>> Furthermore, there is another benefit: now multiple extensions can
>> chain their custom data to bookmark printing. Previously, an extension
>> could have conflicting bookmark output due to which loaded first and
>> overrode commands.bookmarks. Now they can all play nicely together.
>>
>> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
>> index 518b362..797e3c1 100644
>> --- a/mercurial/bookmarks.py
>> +++ b/mercurial/bookmarks.py
>> @@ -765,32 +765,43 @@ def addbookmarks(repo, tr, names, rev=No
>>          activate(repo, newact)
>>      elif cur != tgt and newact == repo._activebookmark:
>>          deactivate(repo)
>>      marks.recordchange(tr)
>>
>> +def _printer(ui, repo, bmarks, **opts):
>
> nit: This actually prints the bookmarks. The name made me think it
> would return an object that could be used for printing.

If you have a better name, then feel free to update in flight!
via Mercurial-devel - June 23, 2017, 7:09 p.m.
I think even _print is better than _printer. Maybe I'll use that, or maybe
something else. _printbookmarks?

On Jun 23, 2017 12:05 PM, "Sean Farley" <sean@farley.io> wrote:

> Martin von Zweigbergk <martinvonz@google.com> writes:
>
> > On Fri, Jun 23, 2017 at 11:33 AM, Sean Farley <sean@farley.io> wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean@farley.io>
> >> # Date 1498004300 25200
> >> #      Tue Jun 20 17:18:20 2017 -0700
> >> # Branch bm-refactor
> >> # Node ID 1e90be070c35665fb3d75657563349650cc523a1
> >> # Parent  5caa40d7fa1be3ff13a2e80a91234809cff5307a
> >> bookmarks: factor method _printer out of for loop in printbookmarks
> >>
> >> This allows even further customization via extensions for printing
> >> bookmarks. For example, in hg-git this would allow printing remote refs
> >> by just modifying the 'bmarks' parameter instead of reimplementing the
> >> old commands.bookmarks method.
> >>
> >> Furthermore, there is another benefit: now multiple extensions can
> >> chain their custom data to bookmark printing. Previously, an extension
> >> could have conflicting bookmark output due to which loaded first and
> >> overrode commands.bookmarks. Now they can all play nicely together.
> >>
> >> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
> >> index 518b362..797e3c1 100644
> >> --- a/mercurial/bookmarks.py
> >> +++ b/mercurial/bookmarks.py
> >> @@ -765,32 +765,43 @@ def addbookmarks(repo, tr, names, rev=No
> >>          activate(repo, newact)
> >>      elif cur != tgt and newact == repo._activebookmark:
> >>          deactivate(repo)
> >>      marks.recordchange(tr)
> >>
> >> +def _printer(ui, repo, bmarks, **opts):
> >
> > nit: This actually prints the bookmarks. The name made me think it
> > would return an object that could be used for printing.
>
> If you have a better name, then feel free to update in flight!
>
Sean Farley - June 23, 2017, 7:11 p.m.
Martin von Zweigbergk <martinvonz@google.com> writes:

> I think even _print is better than _printer. Maybe I'll use that, or maybe
> something else. _printbookmarks?

_printbookmarks is fine with me.

Patch

diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
index 518b362..797e3c1 100644
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -765,32 +765,43 @@  def addbookmarks(repo, tr, names, rev=No
         activate(repo, newact)
     elif cur != tgt and newact == repo._activebookmark:
         deactivate(repo)
     marks.recordchange(tr)
 
+def _printer(ui, repo, bmarks, **opts):
+    """private method to print bookmarks
+
+    Provides a way for extensions to control how bookmarks are printed (e.g.
+    prepend or postpend names)
+    """
+    fm = ui.formatter('bookmarks', opts)
+    hexfn = fm.hexfunc
+    if len(bmarks) == 0 and fm.isplain():
+        ui.status(_("no bookmarks set\n"))
+    for bmark, (n, prefix, label) in sorted(bmarks.iteritems()):
+        fm.startitem()
+        if not ui.quiet:
+            fm.plain(' %s ' % prefix, label=label)
+        fm.write('bookmark', '%s', bmark, label=label)
+        pad = " " * (25 - encoding.colwidth(bmark))
+        fm.condwrite(not ui.quiet, 'rev node', pad + ' %d:%s',
+                     repo.changelog.rev(n), hexfn(n), label=label)
+        fm.data(active=(activebookmarklabel in label))
+        fm.plain('\n')
+    fm.end()
+
 def printbookmarks(ui, repo, **opts):
     """print bookmarks to a formatter
 
     Provides a way for extensions to control how bookmarks are printed.
     """
-    fm = ui.formatter('bookmarks', opts)
-    hexfn = fm.hexfunc
     marks = repo._bookmarks
-    if len(marks) == 0 and fm.isplain():
-        ui.status(_("no bookmarks set\n"))
+    bmarks = {}
     for bmark, n in sorted(marks.iteritems()):
         active = repo._activebookmark
         if bmark == active:
             prefix, label = '*', activebookmarklabel
         else:
             prefix, label = ' ', ''
 
-        fm.startitem()
-        if not ui.quiet:
-            fm.plain(' %s ' % prefix, label=label)
-        fm.write('bookmark', '%s', bmark, label=label)
-        pad = " " * (25 - encoding.colwidth(bmark))
-        fm.condwrite(not ui.quiet, 'rev node', pad + ' %d:%s',
-                     repo.changelog.rev(n), hexfn(n), label=label)
-        fm.data(active=(bmark == active))
-        fm.plain('\n')
-    fm.end()
+        bmarks[bmark] = (n, prefix, label)
+    _printer(ui, repo, bmarks, **opts)