diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -789,6 +789,24 @@ def bookmark(ui, repo, mark=None, rev=No marks = repo._bookmarks cur = repo.changectx('.').node() + def checkformat(mark): + if "\n" in mark: + raise util.Abort(_("bookmark name cannot contain newlines")) + mark = mark.strip() + if not mark: + raise util.Abort(_("bookmark names cannot consist entirely of " + "whitespace")) + return mark + + def checkconflict(repo, mark, force=False): + if mark in marks and not force: + raise util.Abort(_("bookmark '%s' already exists " + "(use -f to force)") % mark) + if ((mark in repo.branchmap() or mark == repo.dirstate.branch()) + and not force): + raise util.Abort( + _("a bookmark cannot have the name of an existing branch")) + if delete: if mark is None: raise util.Abort(_("bookmark name required")) @@ -801,13 +819,12 @@ def bookmark(ui, repo, mark=None, rev=No return if rename: + if mark is None: + raise util.Abort(_("new bookmark name required")) + mark = checkformat(mark) if rename not in marks: raise util.Abort(_("bookmark '%s' does not exist") % rename) - if mark in marks and not force: - raise util.Abort(_("bookmark '%s' already exists " - "(use -f to force)") % mark) - if mark is None: - raise util.Abort(_("new bookmark name required")) + checkconflict(repo, mark, force) marks[mark] = marks[rename] if repo._bookmarkcurrent == rename and not inactive: bookmarks.setcurrent(repo, mark) @@ -816,22 +833,11 @@ def bookmark(ui, repo, mark=None, rev=No return if mark is not None: - if "\n" in mark: - raise util.Abort(_("bookmark name cannot contain newlines")) - mark = mark.strip() - if not mark: - raise util.Abort(_("bookmark names cannot consist entirely of " - "whitespace")) + mark = checkformat(mark) if inactive and mark == repo._bookmarkcurrent: bookmarks.setcurrent(repo, None) return - if mark in marks and not force: - raise util.Abort(_("bookmark '%s' already exists " - "(use -f to force)") % mark) - if ((mark in repo.branchmap() or mark == repo.dirstate.branch()) - and not force): - raise util.Abort( - _("a bookmark cannot have the name of an existing branch")) + checkconflict(repo, mark, force) if rev: marks[mark] = scmutil.revsingle(repo, rev).node() else: diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t --- a/tests/test-bookmarks.t +++ b/tests/test-bookmarks.t @@ -217,12 +217,31 @@ reject bookmark name with newline abort: bookmark name cannot contain newlines [255] + $ hg bookmark -m Z ' + > ' + abort: bookmark name cannot contain newlines + [255] + bookmark with existing name $ hg bookmark Z abort: bookmark 'Z' already exists (use -f to force) [255] + $ hg bookmark -m Y Z + abort: bookmark 'Z' already exists (use -f to force) + [255] + +bookmark with name of branch + + $ hg bookmark default + abort: a bookmark cannot have the name of an existing branch + [255] + + $ hg bookmark -m Y default + abort: a bookmark cannot have the name of an existing branch + [255] + force bookmark with existing name $ hg bookmark -f Z @@ -247,6 +266,10 @@ bookmark name with whitespace only abort: bookmark names cannot consist entirely of whitespace [255] + $ hg bookmark -m Y ' ' + abort: bookmark names cannot consist entirely of whitespace + [255] + invalid bookmark $ hg bookmark 'foo:bar'