bug#38282: 26.3; goto-line should not share input history with other commands

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

Severity: wishlist

When using goto-line (M-g M-g), I usually tend to jump to lines which I
have jumped to in the past. Because goto-line shares input history with
other commands (like `read-from-minibuffer'), sometimes these numbers
get buried among strings that I have entered for other commands. I think
it would make sense to give goto-line a separate input history to make
finding past lines easier.

bug#38282: 26.3; goto-line should not share input history with other commands

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> When using goto-line (M-g M-g), I usually tend to jump to lines which I
> have jumped to in the past. Because goto-line shares input history with
> other commands (like `read-from-minibuffer'), sometimes these numbers
> get buried among strings that I have entered for other commands. I think
> it would make sense to give goto-line a separate input history to make
> finding past lines easier.

Yes, the Emacs behaviour here has annoyed me, too.

`goto-line' basically just calls `read-number' (which calls
`read-from-minibuffer' with the "default" nil history) -- I wonder
whether it would make sense to have a separate history for `read-number'
so that all numbers that are read share a history?  Or is that too
drastic?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38282: 26.3; goto-line should not share input history with other commands

[Robert Pluim]

>>>>> On Thu, 21 Nov 2019 14:51:18 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Federico Tedin <federicotedin@gmail.com> writes:
    >> When using goto-line (M-g M-g), I usually tend to jump to lines which I
    >> have jumped to in the past. Because goto-line shares input history with
    >> other commands (like `read-from-minibuffer'), sometimes these numbers
    >> get buried among strings that I have entered for other commands. I think
    >> it would make sense to give goto-line a separate input history to make
    >> finding past lines easier.

    Lars> Yes, the Emacs behaviour here has annoyed me, too.

    Lars> `goto-line' basically just calls `read-number' (which calls
    Lars> `read-from-minibuffer' with the "default" nil history) -- I wonder
    Lars> whether it would make sense to have a separate history for `read-number'
    Lars> so that all numbers that are read share a history?  Or is that too
    Lars> drastic?

If debbugs-gnu-bugs et al uses read-number, Iʼm all in favour.

Robert

bug#38282: 26.3; goto-line should not share input history with other commands

[Michael Albinus]

Robert Pluim <rpluim@gmail.com> writes:

> If debbugs-gnu-bugs et al uses read-number, Iʼm all in favour.

If you could marry read-number and completing-read-multiple - patches
welcome :-)

(Maybe it is sufficient to tweak HIST in the completing-read-multiple call).

> Robert

Best regards, Michael.

bug#38282: 26.3; goto-line should not share input history with other commands

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Robert Pluim <rpluim@gmail.com> writes:
>
>> If debbugs-gnu-bugs et al uses read-number, Iʼm all in favour.
>
> If you could marry read-number and completing-read-multiple - patches
> welcome :-)

Hm.  I've never looked at completing-read-multiple before (I didn't know
that it existed), but I don't see the connection?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

> Yes, the Emacs behaviour here has annoyed me, too.

> `goto-line' basically just calls `read-number' (which calls
> `read-from-minibuffer' with the "default" nil history) -- I wonder
> whether it would make sense to have a separate history for `read-number'
> so that all numbers that are read share a history?  Or is that too
> drastic?

I started working on this yesterday! My initial plan was to have a
separate history for `read-number', and then I thought that it might be
even better to have the history be buffer-local as well; because line
numbers from one buffer don't really make sense on another.

After I tried some stuff I came across a problem and posted it on the
Emacs SO:
https://emacs.stackexchange.com/questions/53892/buffer-local-input-history-for-read-from-minibuffer

It seems like `read-from-minibuffer' doesn't like having its HIST
parameter be buffer-local (but I may have misunderstood the problem).
I'm going to look into it more on depth these days.

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

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

> It seems like `read-from-minibuffer' doesn't like having its HIST
> parameter be buffer-local (but I may have misunderstood the problem).
> I'm going to look into it more on depth these days.

Here's my first attempt at implementing this - I managed to get around
the `read-from-minibuffer' problem with some help from the Emacs
StackOverflow (but created a separate bug report for it:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38317). I'm attaching a
small patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2882 bytes --]

From 3994fa53d0cf210e7f109dc2059ee78a96215547 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Thu, 21 Nov 2019 23:01:23 +0100
Subject: [PATCH 1/1] Make input history buffer-local for 'goto-line'

* lisp/simple.el (goto-line-history): New buffer-local variable which
holds the input history for 'goto-line'.
(goto-line): Use the new variable as input history (Bug#38282).
* etc/NEWS: Announce changes.
---
 etc/NEWS       |  5 +++++
 lisp/simple.el | 19 +++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 7a51106add..0bbb40dfa2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -589,6 +589,11 @@ If the region is active, the command joins all the lines in the
 region.  When there's no active region, the command works on the
 current and the previous or the next line, as before.
 
+---
+** 'goto-line' now keeps a separate input history for each buffer.
+Additionally, the line number input history will be kept separate from
+the input history for other commands.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 27.1
 
diff --git a/lisp/simple.el b/lisp/simple.el
index c61ccd511c..dd8e8a291a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1212,6 +1212,11 @@ mark-whole-buffer
 
 ;; Counting lines, one way or another.
 
+(defvar goto-line-history nil
+  "Input history for `goto-line'.")
+
+(make-variable-buffer-local 'goto-line-history)
+
 (defun goto-line (line &optional buffer)
   "Go to LINE, counting from line 1 at beginning of buffer.
 If called interactively, a numeric prefix argument specifies
@@ -1253,7 +1258,14 @@ goto-line
 	    (buffer-prompt
 	     (if buffer
 		 (concat " in " (buffer-name buffer))
-	       "")))
+	       ""))
+            ;; It would be better to use `goto-line-history' as a HIST
+            ;; parameter to `read-from-minibuffer' (through
+            ;; `read-number'), but using buffer-local variables
+            ;; doesn't work for that purpose. (Bug#38317)
+            (minibuffer-history
+             (buffer-local-value 'goto-line-history (or buffer
+                                                        (current-buffer)))))
        ;; Read the argument, offering that number (if any) as default.
        (list (read-number (format "Goto line%s: " buffer-prompt)
                           (list default (line-number-at-pos)))
@@ -1271,7 +1283,10 @@ goto-line
     (goto-char (point-min))
     (if (eq selective-display t)
 	(re-search-forward "[\n\C-m]" nil 'end (1- line))
-      (forward-line (1- line)))))
+      (forward-line (1- line))))
+  ;; Save the line number in the input history list.
+  (with-current-buffer (or buffer (current-buffer))
+    (add-to-history 'goto-line-history (int-to-string line))))
 
 (defun count-words-region (start end &optional arg)
   "Count the number of words in the region.
-- 
2.17.1

bug#38282: 26.3; goto-line should not share input history with other commands

[Eli Zaretskii]

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Thu, 21 Nov 2019 23:06:14 +0100
> Cc: 38282@debbugs.gnu.org, Robert Pluim <rpluim@gmail.com>,
>  Michael Albinus <michael.albinus@gmx.de>
> 
> Here's my first attempt at implementing this - I managed to get around
> the `read-from-minibuffer' problem with some help from the Emacs
> StackOverflow (but created a separate bug report for it:
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38317). I'm attaching a
> small patch.

Thanks, but please also update the ELisp manual, where the history
variables are documented in "Minibuffer History".

bug#38282: 26.3; goto-line should not share input history with other commands

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> +            ;; It would be better to use `goto-line-history' as a HIST
> +            ;; parameter to `read-from-minibuffer' (through
> +            ;; `read-number'), but using buffer-local variables
> +            ;; doesn't work for that purpose. (Bug#38317)
> +            (minibuffer-history
> +             (buffer-local-value 'goto-line-history (or buffer
> +                                                        (current-buffer)))))

I think a per-buffer history for goto-line makes sense, but I was also
wondering whether read-number should have its own separate history, too.

This would, I think, not interoperate well with that (that is, if
`read-number' passes a different variable to read-from-minibuffer than
nil).

So I think a better solution would be to fix the problem with
buffer-local variables not working in read-from-minibuffer first, and
then we could extend read-number with a history parameter instead of
hacking around the problem this way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

> I think a per-buffer history for goto-line makes sense, but I was also
> wondering whether read-number should have its own separate history, too.
>
> This would, I think, not interoperate well with that (that is, if
> `read-number' passes a different variable to read-from-minibuffer than
> nil).
>
> So I think a better solution would be to fix the problem with
> buffer-local variables not working in read-from-minibuffer first, and
> then we could extend read-number with a history parameter instead of
> hacking around the problem this way.

No problem, I'll take a shot at solving the `read-from-minubuffer'
issue first.

After that's done, and after adding the HIST parameter to `read-number',
what should happen if `read-number' is called with HIST as nil? Should
it use its own history variable? (Which probably won't be buffer-local,
I imagine)

bug#38282: 26.3; goto-line should not share input history with other commands

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> After that's done, and after adding the HIST parameter to `read-number',
> what should happen if `read-number' is called with HIST as nil? Should
> it use its own history variable? (Which probably won't be buffer-local,
> I imagine)

I think it makes sense for read-number to use its own history variable,
but it should not be buffer local.  Most inputs are not naturally buffer
local.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

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

> I think it makes sense for read-number to use its own history variable,
> but it should not be buffer local.  Most inputs are not naturally buffer
> local.

No problem - I've added an input history variable for `read-number', and
a buffer-local one for `goto-line'. I'm attaching a patch with my
changes.

- Fede


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2720 bytes --]

From 249ccb59be23d94018a01b4f8b1577356732d1da Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Fri, 6 Dec 2019 23:07:27 +0100
Subject: [PATCH 1/1] Make goto-line keep a separate input history per buffer

* lisp/simple.el (goto-line-history): New history variable.
(goto-line): Use new (buffer-local) variable as input history.
* lisp/subr.el (read-number-history): New history variable.
(read-number): Use the new variable as default input history.
---
 lisp/simple.el | 7 ++++++-
 lisp/subr.el   | 9 +++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 67ddab3d34..c14718de1c 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1212,6 +1212,10 @@ mark-whole-buffer
 
 ;; Counting lines, one way or another.
 
+(defvar goto-line-history nil
+  "History of values entered with `goto-line'.")
+(make-variable-buffer-local 'goto-line-history)
+
 (defun goto-line (line &optional buffer)
   "Go to LINE, counting from line 1 at beginning of buffer.
 If called interactively, a numeric prefix argument specifies
@@ -1256,7 +1260,8 @@ goto-line
 	       "")))
        ;; Read the argument, offering that number (if any) as default.
        (list (read-number (format "Goto line%s: " buffer-prompt)
-                          (list default (line-number-at-pos)))
+                          (list default (line-number-at-pos))
+                          'goto-line-history)
 	     buffer))))
   ;; Switch to the desired buffer, one way or another.
   (if buffer
diff --git a/lisp/subr.el b/lisp/subr.el
index de7d919abf..8f7a9128fa 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2518,10 +2518,15 @@ read-passwd
               ;; And of course, don't keep the sensitive data around.
               (erase-buffer))))))))
 
-(defun read-number (prompt &optional default)
+(defvar read-number-history nil
+  "The default history for the `read-number' function.")
+
+(defun read-number (prompt &optional default hist)
   "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.
+HIST specifies a history list variable.  See `read-from-minibuffer'
+for details of the HIST argument.
 This function is used by the `interactive' code letter `n'."
   (let ((n nil)
 	(default1 (if (consp default) (car default) default)))
@@ -2535,7 +2540,7 @@ read-number
     (while
 	(progn
 	  (let ((str (read-from-minibuffer
-		      prompt nil nil nil nil
+		      prompt nil nil nil (or hist 'read-number-history)
 		      (when default
 			(if (consp default)
 			    (mapcar 'number-to-string (delq nil default))
-- 
2.17.1

bug#38282: 26.3; goto-line should not share input history with other commands

[Eli Zaretskii]

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Fri, 06 Dec 2019 23:14:15 +0100
> Cc: 38282@debbugs.gnu.org
> 
> > I think it makes sense for read-number to use its own history variable,
> > but it should not be buffer local.  Most inputs are not naturally buffer
> > local.
> 
> No problem - I've added an input history variable for `read-number', and
> a buffer-local one for `goto-line'. I'm attaching a patch with my
> changes.

Thanks, I think this variable should be mentioned in the ELisp manual
(in the "Minibuffer History" node) and in NEWS.

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

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

> Thanks, I think this variable should be mentioned in the ELisp manual
> (in the "Minibuffer History" node) and in NEWS.

Sorry, forgot to do that. I've now documented both new variables (the
one for `read-number' and the one for `goto-line'). I'm attaching a new
patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4406 bytes --]

From 67e2d243473c52c1b52102203cf069dbed4165cc Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Tue, 17 Dec 2019 22:13:55 +0100
Subject: [PATCH 1/1] Make goto-line keep a separate input history per buffer

* lisp/simple.el (goto-line-history): New history variable.
(goto-line): Use new (buffer-local) variable as input
history (Bug#38282).
* lisp/subr.el (read-number-history): New history variable.
(read-number): Use the new variable as default input history.
* doc/lispref/minibuf.texi (Minibuffer History): Document
read-number-history and goto-line-history variables.
* etc/NEWS: Announce changes.
---
 doc/lispref/minibuf.texi |  9 +++++++++
 etc/NEWS                 | 11 +++++++++++
 lisp/simple.el           |  7 ++++++-
 lisp/subr.el             |  9 +++++++--
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index dde30ce67f..2c2ef9747b 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -645,6 +645,15 @@ Minibuffer History
 @code{read-variable}.
 @end defvar
 
+@defvar read-number-history
+A history list for numbers read by @code{read-number}.
+@end defvar
+
+@defvar goto-line-history
+A history list for arguments to @code{goto-line}.  This variable is
+buffer local.
+@end defvar
+
 @c Less common: coding-system-history, input-method-history,
 @c command-history, grep-history, grep-find-history,
 @c read-envvar-name-history, setenv-history, yes-or-no-p-history.
diff --git a/etc/NEWS b/etc/NEWS
index cf4e705a52..dc17445757 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -533,6 +533,17 @@ The HIST argument of 'read-from-minibuffer' now works correctly with
 buffer-local variables.  This means that different buffers can have
 their own separated input history list if desired.
 
++++
+** 'read-number' now has its own history variable.
+Additionally, the function now accepts a HIST argument which can be
+used to specify a custom history variable.
+
++++
+** Input history for 'goto-line' is now local to every buffer.
+Each buffer will keep a separate history of line numbers used with
+'goto-line'.  This should help making faster the process of finding
+line numbers that were previously jumped to.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/lisp/simple.el b/lisp/simple.el
index 6d5030073b..6219986da0 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1212,6 +1212,10 @@ mark-whole-buffer
 
 ;; Counting lines, one way or another.
 
+(defvar goto-line-history nil
+  "History of values entered with `goto-line'.")
+(make-variable-buffer-local 'goto-line-history)
+
 (defun goto-line (line &optional buffer)
   "Go to LINE, counting from line 1 at beginning of buffer.
 If called interactively, a numeric prefix argument specifies
@@ -1256,7 +1260,8 @@ goto-line
 	       "")))
        ;; Read the argument, offering that number (if any) as default.
        (list (read-number (format "Goto line%s: " buffer-prompt)
-                          (list default (line-number-at-pos)))
+                          (list default (line-number-at-pos))
+                          'goto-line-history)
 	     buffer))))
   ;; Switch to the desired buffer, one way or another.
   (if buffer
diff --git a/lisp/subr.el b/lisp/subr.el
index ed55853bb2..f5b7c98f50 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2518,10 +2518,15 @@ read-passwd
               ;; And of course, don't keep the sensitive data around.
               (erase-buffer))))))))
 
-(defun read-number (prompt &optional default)
+(defvar read-number-history nil
+  "The default history for the `read-number' function.")
+
+(defun read-number (prompt &optional default hist)
   "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.
+HIST specifies a history list variable.  See `read-from-minibuffer'
+for details of the HIST argument.
 This function is used by the `interactive' code letter `n'."
   (let ((n nil)
 	(default1 (if (consp default) (car default) default)))
@@ -2535,7 +2540,7 @@ read-number
     (while
 	(progn
 	  (let ((str (read-from-minibuffer
-		      prompt nil nil nil nil
+		      prompt nil nil nil (or hist 'read-number-history)
 		      (when default
 			(if (consp default)
 			    (mapcar 'number-to-string (delq nil default))
-- 
2.17.1

bug#38282: 26.3; goto-line should not share input history with other commands

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> Sorry, forgot to do that. I've now documented both new variables (the
> one for `read-number' and the one for `goto-line'). I'm attaching a new
> patch.

Thanks; I've now applied this to Emacs 28.  (Only bug fixes are going
into Emacs 27 now, and as this is a new feature, it'll have to wait
until the next release cycle.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38282: 26.3; goto-line should not share input history with other commands

[Federico Tedin]

> Thanks; I've now applied this to Emacs 28.  (Only bug fixes are going
> into Emacs 27 now, and as this is a new feature, it'll have to wait
> until the next release cycle.)

Thanks! And merry Xmas.
- Fede

bug#38282: 26.3; goto-line should not share input history with other commands

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>>> If debbugs-gnu-bugs et al uses read-number, Iʼm all in favour.
>>
>> If you could marry read-number and completing-read-multiple - patches
>> welcome :-)
>
> Hm.  I've never looked at completing-read-multiple before (I didn't know
> that it existed), but I don't see the connection?

debbugs-gnu-bugs uses completing-read-multiple, Robert has asked to use
read-number there. That's why I've mentioned it.

Best regards, Michael.

bug#38282: 26.3; goto-line should not share input history with other commands

[Robert Pluim]

>>>>> On Fri, 22 Nov 2019 08:57:52 +0100, Michael Albinus <michael.albinus@gmx.de> said:

    Michael> Lars Ingebrigtsen <larsi@gnus.org> writes:
    >>>> If debbugs-gnu-bugs et al uses read-number, Iʼm all in favour.
    >>> 
    >>> If you could marry read-number and completing-read-multiple - patches
    >>> welcome :-)
    >> 
    >> Hm.  I've never looked at completing-read-multiple before (I didn't know
    >> that it existed), but I don't see the connection?

    Michael> debbugs-gnu-bugs uses completing-read-multiple, Robert has asked to use
    Michael> read-number there. That's why I've mentioned it.

Well, I assumed it did. But it doesnʼt.

Robert