bug#25032: 25.1; `bookmark-set-internal', `bookmark-set-no-overwrite'

[Stefan Kangas <stefan@marxist.se>]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> > +If OVERWRITE-OR-PUSH is nil, then raise an error if there is
>
> We use "signal an error".

Fixed.

> > +already a bookmark named NAME; if `overwrite', then replace any
> > +existing bookmark if there is one; if `push' then push the new
> > +bookmark onto the bookmark alist.
>
> It is generally better to precede a description of several optional
> behaviors with a short summary:
>
>   OVERWRITE-OR-PUSH controls what happens if there is already a
>   bookmark by that NAME: nil means signal an error, `overwrite' means
>   replace the existing bookmark, `push' means ...

Fixed.  Although I chose the wording "with the same name" rather than
"by that NAME", since the name could also come from the prompt.

Thanks for the review; attached an updated version.

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-lisp-bookmark.el-bookmark-set-internal-Doc-fix.-Bug-2.patch --]
[-- Type: application/octet-stream, Size: 2199 bytes --]

From 7b72c3642f4cc017b1477b0377fb17526f0dc0f8 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Tue, 2 Jul 2019 07:09:13 +0200
Subject: [PATCH] * lisp/bookmark.el (bookmark-set-internal): Doc fix.
 (Bug#25032)

---
 lisp/bookmark.el | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index bbef0a927d..e190649fd1 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -763,18 +763,23 @@ bookmark-minibuffer-read-name-map
     map))
 
 (defun bookmark-set-internal (prompt name overwrite-or-push)
-  "Interactively set a bookmark named NAME at the current location.
-
-Begin the interactive prompt with PROMPT, followed by a space, a
-generated default name in parentheses, a colon and a space.
-
-If OVERWRITE-OR-PUSH is nil, then error if there is already a
-bookmark named NAME; if `overwrite', then replace any existing
-bookmark if there is one; if `push' then push the new bookmark
-onto the bookmark alist.  The `push' behavior means that among
-bookmarks named NAME, this most recently set one becomes the one in
-effect, but the others are still there, in order, if the topmost one
-is ever deleted."
+  "Set a bookmark using specified NAME or prompting with PROMPT.
+The bookmark is set at the current location.
+
+If NAME is non-nil, use it as the name of the new bookmark.  In
+this case, the value of PROMPT is ignored.
+
+Otherwise, prompt the user for the bookmark name.  Begin the
+interactive prompt with PROMPT, followed by a space, a generated
+default name in parentheses, a colon and a space.
+
+OVERWRITE-OR-PUSH controls what happens if there is already a
+bookmark with the same name: nil means signal an error;
+`overwrite' means replace any existing bookmark; `push' means
+push the new bookmark onto the bookmark alist.  The `push'
+behavior means that among bookmarks with the same name, this most
+recently set one becomes the one in effect, but the others are
+still there, in order, if the topmost one is ever deleted."
   (unwind-protect
        (let* ((record (bookmark-make-record))
               ;; `defaults' is a transient element of the
-- 
2.21.0