Patchwork [v2] bookmarks: reject bookmark names that unambiguously resolve to a node (BC)

login
register
mail settings
Submitter Augie Fackler
Date May 25, 2017, midnight
Message ID <97e663d08b2795306c36.1495670419@augie-macbookpro2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/20888/
State Accepted
Headers show

Comments

Augie Fackler - May 25, 2017, midnight
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1495495092 14400
#      Mon May 22 19:18:12 2017 -0400
# Node ID 97e663d08b2795306c36030dd9bb6f88c1945580
# Parent  548478efc46c6147e9c2781cf70477b3461b440d
bookmarks: reject bookmark names that unambiguously resolve to a node (BC)

I just burned myself on this today because I left out the -r in my `hg
bookmark` command, which then left me confused because I didn't notice
the bookmark I created in the wrong place that was silently shadowing
the revision I was trying to check out. Let's warn the user.

This patch only enforces the check on bookmark names 4 characters long
or longer. We can tweak that if we'd like, I selected that since
that's the fewest characters shortest will use in the templater
output.

A previous version of this patch rejected such bookmarks. It was
proposed during review (and I agree) that the behavior change for a
bookmark named "cafe" or similar as history accumulated was a little
too weird, but that the warning definitely has merit.
via Mercurial-devel - May 25, 2017, 6:12 a.m.
On Wed, May 24, 2017 at 5:00 PM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1495495092 14400
> #      Mon May 22 19:18:12 2017 -0400
> # Node ID 97e663d08b2795306c36030dd9bb6f88c1945580
> # Parent  548478efc46c6147e9c2781cf70477b3461b440d
> bookmarks: reject bookmark names that unambiguously resolve to a node (BC)

Queuing with s/reject/warn about/ in flight. Thanks, I've also made
the mistake (of forgetting -r) several times.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -968,6 +968,11 @@  def bookmark(ui, repo, *names, **opts):
             and not force):
             raise error.Abort(
                 _("a bookmark cannot have the name of an existing branch"))
+        if len(mark) > 3 and mark in repo and not force:
+            repo.ui.warn(
+                _("bookmark %s matches a changeset hash\n"
+                  "(did you leave a -r out of an 'hg bookmark' command?)\n") %
+                mark)
 
     if delete and rename:
         raise error.Abort(_("--delete and --rename are incompatible"))
diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -311,6 +311,15 @@  bookmark with integer name
   abort: cannot use an integer as a name
   [255]
 
+bookmark with a name that matches a node id
+  $ hg bookmark 925d80f479bb db815d6d32e6
+  bookmark 925d80f479bb matches a changeset hash
+  (did you leave a -r out of an 'hg bookmark' command?)
+  bookmark db815d6d32e6 matches a changeset hash
+  (did you leave a -r out of an 'hg bookmark' command?)
+  $ hg bookmark -d 925d80f479bb
+  $ hg bookmark -d db815d6d32e6
+
 incompatible options
 
   $ hg bookmark -m Y -d Z