Patchwork [9,of,9] bookmarks: remove --active in favor of --list

login
register
mail settings
Submitter Yuya Nishihara
Date Sept. 21, 2018, 1:24 p.m.
Message ID <593f95856fad7581150f.1537536251@mimosa>
Download mbox | patch
Permalink /patch/34902/
State Accepted
Headers show

Comments

Yuya Nishihara - Sept. 21, 2018, 1:24 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1536983269 -32400
#      Sat Sep 15 12:47:49 2018 +0900
# Node ID 593f95856fad7581150f52f49dae33087b4191c8
# Parent  323412d960084affba93ec7a23d8436392ba3fa6
bookmarks: remove --active in favor of --list

It's weird that we have both --active and --inactive options meaning
completely different things. Instead of adding a one-off option, let's
document the way to display the active bookmark by using -l/--list.

No deprecated option is added since --active isn't released yet.
Augie Fackler - Sept. 21, 2018, 3:14 p.m.
> On Sep 21, 2018, at 09:24, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1536983269 -32400
> #      Sat Sep 15 12:47:49 2018 +0900
> # Node ID 593f95856fad7581150f52f49dae33087b4191c8
> # Parent  323412d960084affba93ec7a23d8436392ba3fa6
> bookmarks: remove --active in favor of --list

Very nice behavioral cleanup. I'm a big fan, this addresses the discomfort I had with the --active series.

Queued.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -904,7 +904,6 @@  def bisect(ui, repo, rev=None, extra=Non
     ('m', 'rename', '', _('rename a given bookmark'), _('OLD')),
     ('i', 'inactive', False, _('mark a bookmark inactive')),
     ('l', 'list', False, _('list existing bookmarks')),
-    ('', 'active', False, _('display the active bookmark')),
     ] + formatteropts,
     _('hg bookmarks [OPTIONS]... [NAME]...'))
 def bookmark(ui, repo, *names, **opts):
@@ -931,10 +930,6 @@  def bookmark(ui, repo, *names, **opts):
     A bookmark named '@' has the special property that :hg:`clone` will
     check it out by default if it exists.
 
-    The '--active' flag will display the current bookmark or return non-zero,
-    if combined with other action, they will be performed on the active
-    bookmark.
-
     .. container:: verbose
 
       Examples:
@@ -958,14 +953,17 @@  def bookmark(ui, repo, *names, **opts):
       - move the '@' bookmark from another branch::
 
           hg book -f @
+
+      - print only the active bookmark name::
+
+          hg book -ql .
     '''
     opts = pycompat.byteskwargs(opts)
     force = opts.get('force')
     rev = opts.get('rev')
     inactive = opts.get('inactive')  # meaning add/rename to inactive bookmark
 
-    selactions = [k for k in ['delete', 'rename', 'active', 'list']
-                  if opts.get(k)]
+    selactions = [k for k in ['delete', 'rename', 'list'] if opts.get(k)]
     if len(selactions) > 1:
         raise error.Abort(_('--%s and --%s are incompatible')
                           % tuple(selactions[:2]))
@@ -978,11 +976,9 @@  def bookmark(ui, repo, *names, **opts):
     else:
         action = 'list'
 
-    if rev and action in {'delete', 'rename', 'active', 'list'}:
+    if rev and action in {'delete', 'rename', 'list'}:
         raise error.Abort(_("--rev is incompatible with --%s") % action)
-    if names and action == 'active':
-        raise error.Abort(_("NAMES is incompatible with --active"))
-    if inactive and action in {'delete', 'active', 'list'}:
+    if inactive and action in {'delete', 'list'}:
         raise error.Abort(_("--inactive is incompatible with --%s") % action)
     if not names and action in {'add', 'delete'}:
         raise error.Abort(_("bookmark name required"))
@@ -1008,11 +1004,6 @@  def bookmark(ui, repo, *names, **opts):
                     ui.status(_("no active bookmark\n"))
                 else:
                     bookmarks.deactivate(repo)
-    elif action == 'active':
-        book = repo._activebookmark
-        if book is None:
-            return 1
-        ui.write("%s\n" % book, label=bookmarks.activebookmarklabel)
     elif action == 'list':
         names = pycompat.maplist(repo._bookmarks.expandname, names)
         with ui.formatter('bookmarks', opts) as fm:
diff --git a/tests/test-bookmarks-current.t b/tests/test-bookmarks-current.t
--- a/tests/test-bookmarks-current.t
+++ b/tests/test-bookmarks-current.t
@@ -240,8 +240,9 @@  display how "{activebookmark}" template 
   - Y
   - 
 
-  $ hg bookmarks --active
+  $ hg bookmarks -ql .
   Y
   $ hg bookmarks --inactive
-  $ hg bookmarks --active
-  [1]
+  $ hg bookmarks -ql .
+  abort: no active bookmark!
+  [255]
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -250,7 +250,7 @@  Show all commands + options
   archive: no-decode, prefix, rev, type, subrepos, include, exclude
   backout: merge, commit, no-commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user
   bisect: reset, good, bad, skip, extend, command, noupdate
-  bookmarks: force, rev, delete, rename, inactive, list, active, template
+  bookmarks: force, rev, delete, rename, inactive, list, template
   branch: force, clean, rev
   branches: active, closed, template
   bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure