Re: [PATCH] fix goto-line

[Juri Linkov <juri@jurta.org>]

>> 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.