Re: [PATCH] fix goto-line

[Juri Linkov]

>> Just a tiny patch fixing a problem in goto-line, which was passing a
>> string as the second parameter to read-number, triggering an error.
>
> Committed.

I noticed the following areas for improvement:

1. `read-number' already inserts the default value into prompt.
   So the same code is not necessary in `goto-line'.

2. It would be useful to add the current line number to the defaults
   of `goto-line' to allow its easier modification by users.

This is implemented with:

=== modified file 'lisp/simple.el'
--- lisp/simple.el      2011-07-28 14:05:07 +0000
+++ lisp/simple.el      2011-07-29 11:14:04 +0000
@@ -914,11 +914,8 @@ (defun goto-line (line &optional buffer)
                 (concat " in " (buffer-name buffer))
               "")))
        ;; Read the argument, offering that number (if any) as default.
-       (list (read-number (format (if default "Goto line%s (%s): "
-                                    "Goto line%s: ")
-                                  buffer-prompt
-                                  default)
-                          default)
+       (list (read-number (format "Goto line%s: " buffer-prompt)
+                          (list default (line-number-at-pos)))
             buffer))))
   ;; Switch to the desired buffer, one way or another.
   (if buffer


3. `read-number' still doesn't support multiple default values
   like other minibuffer reading functions.  Let's do it.

=== modified file 'lisp/subr.el'
--- lisp/subr.el        2011-07-15 23:59:25 +0000
+++ lisp/subr.el        2011-07-29 11:12:45 +0000
@@ -2113,23 +2113,27 @@ (defun read-number (prompt &optional def
   "Read a numeric value in the minibuffer, prompting with PROMPT.
 DEFAULT specifies a default value to return if the user just types RET.
 The value of DEFAULT is inserted into PROMPT."
-  (let ((n nil))
-    (when default
+  (let ((n nil)
+       (default1 (if (consp default) (car default) default)))
+    (when default1
       (setq prompt
            (if (string-match "\\(\\):[ \t]*\\'" prompt)
-               (replace-match (format " (default %s)" default) t t prompt 1)
+               (replace-match (format " (default %s)" default1) t t prompt 1)
              (replace-regexp-in-string "[ \t]*\\'"
-                                       (format " (default %s) " default)
+                                       (format " (default %s) " default1)
                                        prompt t t))))
     (while
        (progn
-         (let ((str (read-from-minibuffer prompt nil nil nil nil
-                                          (and default
-                                               (number-to-string default)))))
+         (let ((str (read-from-minibuffer
+                     prompt nil nil nil nil
+                     (when default
+                       (if (consp default)
+                           (mapcar 'number-to-string (delq nil default))
+                         (number-to-string default))))))
            (condition-case nil
                (setq n (cond
-                        ((zerop (length str)) default)
-                        ((stringp str) (read str))))
+                        ((zerop (length str)) default1)
+                        ((stringp str) (string-to-number str))))
              (error nil)))
          (unless (numberp n)
            (message "Please enter a number.")


4. In the patch above I replaced `read' with `string-to-number'
   for consistency.