bug#14176: 24.3.50; `bookmark-completing-read': prompt and return value for "" DEFAULT

[Thierry Volpiatto <thierry.volpiatto@gmail.com>]

"Drew Adams" <drew.adams@oracle.com> writes:

> The prompt still shows empty parens: "Bookmark (): ".

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index c1d8a4a..ad1609b 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -437,22 +437,18 @@ the empty string."
 						'string-lessp)
 					(bookmark-all-names)))
     (let* ((completion-ignore-case bookmark-completion-ignore-case)
-	   (default default)
+           (default (unless (string= "" default) default))
 	   (prompt (concat prompt (if default
                                       (format " (%s): " default)
-                                    ": ")))
-	   (str
-	    (completing-read prompt
-			     (lambda (string pred action)
-                               (if (eq action 'metadata)
-                                   '(metadata (category . bookmark))
-                                 (complete-with-action
-                                  action bookmark-alist string pred)))
-			     nil
-			     0
-			     nil
-			     'bookmark-history)))
-      (if (string-equal "" str) default str))))
+                                    ": "))))
+      (completing-read prompt
+                       (lambda (string pred action)
+                         (if (eq action 'metadata)
+                             '(metadata (category . bookmark))
+                             (complete-with-action
+                              action bookmark-alist string pred)))
+                       nil 0 nil 'bookmark-history default))))
+
 
 
 (defmacro bookmark-maybe-historicize-string (string)

> (The handling of a cons DEFAULT is appropriate since Emacs 23.  But that minor
> enhancement is really separate from this bug report.  #1 is just about empty
> parens in the prompt.)

Don't see why DEFAULT would be a cons, a bookmark name is a string, so IMO
DEFAULT should be a string or nil.

>
> Wrt #2:
>
> a. I misspoke a bit.  The behavior was that nil (not "") was returned when
> DEFAULT is nil and the user enters empty input.  IMO, the value returned should
> be "" (which Thierry's patch fixes, BTW).
>
> Given that correction, this (new) behavior should be pointed out in the doc
> string.  That is #2 of the bug report.
>
> IOW, the function should always return a string, and that string should be empty
> ("") if DEFAULT is nil and the user input is empty.

(bookmark-completing-read "test" "")
=> ""
(bookmark-completing-read "test")
=> ""
(bookmark-completing-read "test" "foo")
=> "foo"


> And we should point out this behavior explicitly in the doc string,
> for clarity.

Maybe you can provide a patch ?...

Thanks.

-- 
Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997