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

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

Hi Drew,

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

> 1. Do not insert DEFAULT, in parens, into the PROMPT if DEFAULT is "".
>  
> 2. Doc string should mention the return value ("") for empty input ("")
> when DEFAULT is nil.

Isn't there a bad usage of `completing-read' here, IMO the
DEFAULT arg of `completing-read' should be used instead of:

,----
| (if (string-equal "" str) default str)
`----

Also why is 'default' let-bounded ?

Here a patch:

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index c1d8a4a..8698821 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -437,22 +437,21 @@ the empty string."
 						'string-lessp)
 					(bookmark-all-names)))
     (let* ((completion-ignore-case bookmark-completion-ignore-case)
-	   (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)


    
> In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
>  of 2013-04-02 on ODIEONE
> Bzr revision: 112212 cyd@gnu.org-20130402033331-sqegwhqh7u1o0ars
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> Configured using:
>  `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
>  -IC:/Devel/emacs/build/include --ldflags -LC:/Devel/emacs/build/lib'
>  
>
>
>
>
>

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