bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Attached is a proposed patch that adds input history read/write
capabilities to IELM.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 1869 bytes --]

From 190a88ae55bb4a47de64cb50f2706798a100704b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Wed, 8 Nov 2023 16:08:08 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
---
 lisp/ielm.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lisp/ielm.el b/lisp/ielm.el
index e51c46054b4..618cd033055 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -152,6 +152,10 @@ ielm-header
 \\[describe-mode] for help.\n"
   "Message to display when IELM is started.")
 
+(defvar ielm--history-file-name
+  (locate-user-emacs-file "ielm-history.el")
+  "Name of the file to read/write IELM input history.")
+
 (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
 (defvar-keymap ielm-map
   :doc "Keymap for IELM mode."
@@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm--history-file-name)
+  (setq-local kill-buffer-hook #'comint-write-input-ring)
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
@@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
     (set-marker comint-last-input-start (ielm-pm))
     (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
 
+(unless noninteractive
+  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
+
 (defun ielm-get-old-input nil
   ;; Return the previous input surrounding point
   (save-excursion
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]


-- Simen

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Attached is a proposed patch that adds input history read/write
> capabilities to IELM.

Thanks.  This feature makes sense to me.

- This should be called out in NEWS.
- Does it need any other documentation changes?
- Do we usually provide options to disable this stuff?

> From 190a88ae55bb4a47de64cb50f2706798a100704b Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
> Date: Wed, 8 Nov 2023 16:08:08 +0100
> Subject: [PATCH] Add support for reading/writing IELM input history
>
> * lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
> reading/writing input history.
> (ielm--history-file-name): Name of the file to read/write IELM input
> history.
> ---
>  lisp/ielm.el | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/lisp/ielm.el b/lisp/ielm.el
> index e51c46054b4..618cd033055 100644
> --- a/lisp/ielm.el
> +++ b/lisp/ielm.el
> @@ -152,6 +152,10 @@ ielm-header
>  \\[describe-mode] for help.\n"
>    "Message to display when IELM is started.")
>
> +(defvar ielm--history-file-name
> +  (locate-user-emacs-file "ielm-history.el")

Shouldn't that be .eld?

> +  "Name of the file to read/write IELM input history.")
> +
>  (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
>  (defvar-keymap ielm-map
>    :doc "Keymap for IELM mode."
> @@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
>              #'ielm-indirect-setup-hook 'append t)
>    (setq comint-indirect-setup-function #'emacs-lisp-mode)
>
> +  ;; Input history
> +  (setq-local comint-input-ring-file-name ielm--history-file-name)
> +  (setq-local kill-buffer-hook #'comint-write-input-ring)
> +  (comint-read-input-ring t)
> +
>    ;; A dummy process to keep comint happy. It will never get any input
>    (unless (comint-check-proc (current-buffer))
>      ;; Was cat, but on non-Unix platforms that might not exist, so
> @@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
>      (set-marker comint-last-input-start (ielm-pm))
>      (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
>
> +(unless noninteractive
> +  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
> +
>  (defun ielm-get-old-input nil
>    ;; Return the previous input surrounding point
>    (save-excursion
> --
> 2.39.2
>
>
> -- Simen

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi Stefan, thank you for your response.

Stefan Kangas <stefankangas@gmail.com> writes:

> - This should be called out in NEWS.

Yup, done in the revised patch attached.

> - Does it need any other documentation changes?

I don't think so; IELM is only mentioned briefly in the Emacs manual.

> - Do we usually provide options to disable this stuff?

Not as far as I can tell.

>> +(defvar ielm--history-file-name
>> +  (locate-user-emacs-file "ielm-history.el")
>
> Shouldn't that be .eld?

Hm, the file will contain actual Lisp expressions, so I'd think that .el
is fitting?

-- Simen


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 2282 bytes --]

From 3ac85190a90fd85bb7a960cd4895c3bd4e41b085 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Wed, 8 Nov 2023 16:08:08 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
---
 etc/NEWS     |  5 +++++
 lisp/ielm.el | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 1cdb12c3958..f65d53e4827 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1216,6 +1216,11 @@ chat buffers use by default.
 *** New command 'customize-dirlocals'.
 This command pops up a buffer to edit the settings in ".dir-locals.el".
 
+** IELM
+
+---
+*** IELM now remembers input history between sessions.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/ielm.el b/lisp/ielm.el
index 777aebb70cf..46a2a1799da 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -152,6 +152,10 @@ ielm-header
 \\[describe-mode] for help.\n"
   "Message to display when IELM is started.")
 
+(defvar ielm--history-file-name
+  (locate-user-emacs-file "ielm-history.el")
+  "Name of the file to read/write IELM input history.")
+
 (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
 (defvar-keymap ielm-map
   :doc "Keymap for IELM mode."
@@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm--history-file-name)
+  (setq-local kill-buffer-hook #'comint-write-input-ring)
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
@@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
     (set-marker comint-last-input-start (ielm-pm))
     (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
 
+(unless noninteractive
+  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
+
 (defun ielm-get-old-input nil
   ;; Return the previous input surrounding point
   (save-excursion
-- 
2.39.2

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl <simenheg@runbox.com> writes:

>>> +(defvar ielm--history-file-name
>>> +  (locate-user-emacs-file "ielm-history.el")
>>
>> Shouldn't that be .eld?
>
> Hm, the file will contain actual Lisp expressions, so I'd think that .el
> is fitting?

I'm not so sure.  We don't really intend for users to use it the way
you'd expect from an .el file, like byte-compiling or loading it.

So while these are, hopefully, valid Lisp expressions (barring typos,
which frequently happens at a REPL), it's not really a file containing
meaningful code either.

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> Simen Heggestøyl <simenheg@runbox.com> writes:
>
>>>> +(defvar ielm--history-file-name
>>>> +  (locate-user-emacs-file "ielm-history.el")
>>>
>>> Shouldn't that be .eld?
>>
>> Hm, the file will contain actual Lisp expressions, so I'd think that .el
>> is fitting?
>
> I'm not so sure.  We don't really intend for users to use it the way
> you'd expect from an .el file, like byte-compiling or loading it.
>
> So while these are, hopefully, valid Lisp expressions (barring typos,
> which frequently happens at a REPL), it's not really a file containing
> meaningful code either.

Okay, fair points. Updated patch attached!

-- Simen


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 2283 bytes --]

From a2322770acb6bceab36224989cbbe9b777312997 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Wed, 8 Nov 2023 16:08:08 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
---
 etc/NEWS     |  5 +++++
 lisp/ielm.el | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index c3d777b971f..b62976143d2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1228,6 +1228,11 @@ chat buffers use by default.
 *** New command 'customize-dirlocals'.
 This command pops up a buffer to edit the settings in ".dir-locals.el".
 
+** IELM
+
+---
+*** IELM now remembers input history between sessions.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/ielm.el b/lisp/ielm.el
index 777aebb70cf..6d4a11c1c28 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -152,6 +152,10 @@ ielm-header
 \\[describe-mode] for help.\n"
   "Message to display when IELM is started.")
 
+(defvar ielm--history-file-name
+  (locate-user-emacs-file "ielm-history.eld")
+  "Name of the file to read/write IELM input history.")
+
 (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
 (defvar-keymap ielm-map
   :doc "Keymap for IELM mode."
@@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm--history-file-name)
+  (setq-local kill-buffer-hook #'comint-write-input-ring)
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
@@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
     (set-marker comint-last-input-start (ielm-pm))
     (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
 
+(unless noninteractive
+  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
+
 (defun ielm-get-old-input nil
   ;; Return the previous input surrounding point
   (save-excursion
-- 
2.39.2

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl <simenheg@runbox.com> writes:

> From a2322770acb6bceab36224989cbbe9b777312997 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
> Date: Wed, 8 Nov 2023 16:08:08 +0100
> Subject: [PATCH] Add support for reading/writing IELM input history
>
> * lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
> reading/writing input history.
> (ielm--history-file-name): Name of the file to read/write IELM input
> history.
> ---
>  etc/NEWS     |  5 +++++
>  lisp/ielm.el | 12 ++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index c3d777b971f..b62976143d2 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1228,6 +1228,11 @@ chat buffers use by default.
>  *** New command 'customize-dirlocals'.
>  This command pops up a buffer to edit the settings in ".dir-locals.el".
>
> +** IELM
> +
> +---
> +*** IELM now remembers input history between sessions.
> +
>  \f
>  * New Modes and Packages in Emacs 30.1
>
> diff --git a/lisp/ielm.el b/lisp/ielm.el
> index 777aebb70cf..6d4a11c1c28 100644
> --- a/lisp/ielm.el
> +++ b/lisp/ielm.el
> @@ -152,6 +152,10 @@ ielm-header
>  \\[describe-mode] for help.\n"
>    "Message to display when IELM is started.")
>
> +(defvar ielm--history-file-name
> +  (locate-user-emacs-file "ielm-history.eld")
> +  "Name of the file to read/write IELM input history.")

On closer thought, I think this should be a defcustom.

See `lua-ts-inferior-history' and `sql-input-ring-file-name' for
example.

I also think nil might as well be a valid value which disables history
altogether (maybe users don't want that for some reason, let's say
privacy or whatever).

And the defcustom should be mentioned in NEWS, too.

>  (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
>  (defvar-keymap ielm-map
>    :doc "Keymap for IELM mode."
> @@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
>              #'ielm-indirect-setup-hook 'append t)
>    (setq comint-indirect-setup-function #'emacs-lisp-mode)
>
> +  ;; Input history
> +  (setq-local comint-input-ring-file-name ielm--history-file-name)
> +  (setq-local kill-buffer-hook #'comint-write-input-ring)
> +  (comint-read-input-ring t)
> +
>    ;; A dummy process to keep comint happy. It will never get any input
>    (unless (comint-check-proc (current-buffer))
>      ;; Was cat, but on non-Unix platforms that might not exist, so
> @@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
>      (set-marker comint-last-input-start (ielm-pm))
>      (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
>
> +(unless noninteractive
> +  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
> +

I'm not so sure about this part.  If this is correct, why should it be
defined here, and not, say, in comint.el?  It seems like a separate,
unrelated change.  What am I missing?

>  (defun ielm-get-old-input nil
>    ;; Return the previous input surrounding point
>    (save-excursion
> --
> 2.39.2

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> On closer thought, I think this should be a defcustom.
>
> See `lua-ts-inferior-history' and `sql-input-ring-file-name' for
> example.
>
> I also think nil might as well be a valid value which disables history
> altogether (maybe users don't want that for some reason, let's say
> privacy or whatever).
>
> And the defcustom should be mentioned in NEWS, too.

Thanks, makes sense. Updated in the attached patch!

>>  (defvaralias 'inferior-emacs-lisp-mode-map 'ielm-map)
>>  (defvar-keymap ielm-map
>>    :doc "Keymap for IELM mode."
>> @@ -605,6 +609,11 @@ inferior-emacs-lisp-mode
>>              #'ielm-indirect-setup-hook 'append t)
>>    (setq comint-indirect-setup-function #'emacs-lisp-mode)
>>
>> +  ;; Input history
>> +  (setq-local comint-input-ring-file-name ielm--history-file-name)
>> +  (setq-local kill-buffer-hook #'comint-write-input-ring)
>> +  (comint-read-input-ring t)
>> +
>>    ;; A dummy process to keep comint happy. It will never get any input
>>    (unless (comint-check-proc (current-buffer))
>>      ;; Was cat, but on non-Unix platforms that might not exist, so
>> @@ -631,6 +640,9 @@ inferior-emacs-lisp-mode
>>      (set-marker comint-last-input-start (ielm-pm))
>>      (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
>>
>> +(unless noninteractive
>> +  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
>> +
>
> I'm not so sure about this part.  If this is correct, why should it be
> defined here, and not, say, in comint.el?  It seems like a separate,
> unrelated change.  What am I missing?

Usually comint-derived modes use a process sentinel to listen to some
external process and call `comint-write-input-ring' when the process
exists, which works both the buffer is killed or Emacs itself is quit.

For IELM there is no such process to listen to, so I hook into
`kill-buffer-hook' and `kill-emacs-hook' to achieve the same things.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 2652 bytes --]

From fdfb18c16d1d38a197bc1143d2704aefa7fe9daf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Wed, 8 Nov 2023 16:08:08 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
---
 etc/NEWS     | 11 +++++++++++
 lisp/ielm.el | 15 +++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index bce33f96aee..c20d34b39ad 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1243,6 +1243,17 @@ chat buffers use by default.
 *** New command 'customize-dirlocals'.
 This command pops up a buffer to edit the settings in ".dir-locals.el".
 
+** IELM
+
+---
+*** IELM now remembers input history between sessions.
+
+---
+*** New variable 'ielm-history-file-name'.
+If non-nil, name of the file to read/write IELM input history.  Set to
+nil to revert IELM to the old behavior of not remembering input
+history between sessions.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/ielm.el b/lisp/ielm.el
index 777aebb70cf..cfb51b7a94e 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -110,6 +110,13 @@ ielm-dynamic-multiline-inputs
 such as `edebug-defun' to work with such inputs."
   :type 'boolean)
 
+(defcustom ielm-history-file-name
+  (locate-user-emacs-file "ielm-history.eld")
+  "If non-nil, name of the file to read/write IELM input history."
+  :type '(choice (const :tag "nil" nil)
+		 file)
+  :version "30.1")
+
 (defvaralias 'inferior-emacs-lisp-mode-hook 'ielm-mode-hook)
 (defcustom ielm-mode-hook nil
   "Hooks to be run when IELM (`inferior-emacs-lisp-mode') is started."
@@ -605,6 +612,11 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm-history-file-name)
+  (setq-local kill-buffer-hook #'comint-write-input-ring)
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
@@ -631,6 +643,9 @@ inferior-emacs-lisp-mode
     (set-marker comint-last-input-start (ielm-pm))
     (set-process-filter (get-buffer-process (current-buffer)) 'comint-output-filter)))
 
+(unless noninteractive
+  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
+
 (defun ielm-get-old-input nil
   ;; Return the previous input surrounding point
   (save-excursion
-- 
2.42.0


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]


-- Simen

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl <simenheg@runbox.com> writes:

>>> +(unless noninteractive
>>> +  (add-hook 'kill-emacs-hook #'comint-write-input-ring))
>>
>> I'm not so sure about this part.  If this is correct, why should it be
>> defined here, and not, say, in comint.el?  It seems like a separate,
>> unrelated change.  What am I missing?
>
> Usually comint-derived modes use a process sentinel to listen to some
> external process and call `comint-write-input-ring' when the process
> exists, which works both the buffer is killed or Emacs itself is quit.
>
> For IELM there is no such process to listen to, so I hook into
> `kill-buffer-hook' and `kill-emacs-hook' to achieve the same things.

I'm not against adding a hook to `kill-emacs-hook', but it should be
added when you start IELM and not on top-level.  We want to avoid any
side effects from merely loading a file.

I don't think this will work in its current form either: AFAICT,
`comint-write-input-ring' saves it for the current buffer, and that's
not guaranteed to be the *ielm* buffer when `kill-emacs-hook' runs.

So you'd want to first switch to that buffer, I think.  It could also
have been renamed meanwhile, so I think you want to save the actual
buffer and not it's name (i.e. use `get-buffer').

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> I'm not against adding a hook to `kill-emacs-hook', but it should be
> added when you start IELM and not on top-level.  We want to avoid any
> side effects from merely loading a file.

Fixed.

> I don't think this will work in its current form either: AFAICT,
> `comint-write-input-ring' saves it for the current buffer, and that's
> not guaranteed to be the *ielm* buffer when `kill-emacs-hook' runs.
>
> So you'd want to first switch to that buffer, I think.  It could also
> have been renamed meanwhile, so I think you want to save the actual
> buffer and not it's name (i.e. use `get-buffer').

Right, good catch. New attempt in the attached patch!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 3190 bytes --]

From 738801b3126e78cf893d58e47fa156aeeec84cfc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Tue, 16 Jan 2024 08:21:41 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
(ielm--exit): Holds a function to call when Emacs is killed to write
out the input history.
(ielm--input-history-writer): Helper function for writing the IELM
input history out to file.
---
 etc/NEWS     | 11 +++++++++++
 lisp/ielm.el | 29 +++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 735a05f6579..b87ba93f456 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1254,6 +1254,17 @@ chat buffers use by default.
 *** New command 'customize-dirlocals'.
 This command pops up a buffer to edit the settings in ".dir-locals.el".
 
+** IELM
+
+---
+*** IELM now remembers input history between sessions.
+
+---
+*** New variable 'ielm-history-file-name'.
+If non-nil, name of the file to read/write IELM input history.  Set to
+nil to revert IELM to the old behavior of not remembering input
+history between sessions.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/ielm.el b/lisp/ielm.el
index 777aebb70cf..54da62c4359 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -110,6 +110,13 @@ ielm-dynamic-multiline-inputs
 such as `edebug-defun' to work with such inputs."
   :type 'boolean)
 
+(defcustom ielm-history-file-name
+  (locate-user-emacs-file "ielm-history.eld")
+  "If non-nil, name of the file to read/write IELM input history."
+  :type '(choice (const :tag "nil" nil)
+                 file)
+  :version "30.1")
+
 (defvaralias 'inferior-emacs-lisp-mode-hook 'ielm-mode-hook)
 (defcustom ielm-mode-hook nil
   "Hooks to be run when IELM (`inferior-emacs-lisp-mode') is started."
@@ -503,6 +510,17 @@ ielm--expand-ellipsis
     (funcall pp-default-function beg end)
     end))
 
+;;; Input history
+
+(defvar ielm--exit
+  "Function to call when Emacs is killed.")
+
+(defun ielm--input-history-writer (buf)
+  "Return a function writing IELM input history to BUF."
+  (lambda ()
+    (with-current-buffer buf
+      (comint-write-input-ring))))
+
 ;;; Major mode
 
 (define-derived-mode inferior-emacs-lisp-mode comint-mode "IELM"
@@ -605,6 +623,17 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm-history-file-name)
+  (setq-local ielm--exit (ielm--input-history-writer (current-buffer)))
+  (setq-local kill-buffer-hook
+              (lambda ()
+                (funcall ielm--exit)
+                (remove-hook 'kill-emacs-hook ielm--exit)))
+  (unless noninteractive
+    (add-hook 'kill-emacs-hook ielm--exit))
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 10 bytes --]


-- Simen

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl <simenheg@runbox.com> writes:

> Right, good catch. New attempt in the attached patch!

Thanks, please find some comments below.

> +---
> +*** IELM now remembers input history between sessions.
> +
> +---
> +*** New variable 'ielm-history-file-name'.
> +If non-nil, name of the file to read/write IELM input history.  Set to
> +nil to revert IELM to the old behavior of not remembering input
> +history between sessions.

I would probably make this into one entry, like so:

*** IELM now remembers input history between sessions.
The new user option 'ielm-history-file-name' is the name of the file
where IELM input history will be saved.  Customize it to nil to revert
to the old behavior of not remembering input history between sessions.

> +(defcustom ielm-history-file-name
> +  (locate-user-emacs-file "ielm-history.eld")
> +  "If non-nil, name of the file to read/write IELM input history."
> +  :type '(choice (const :tag "nil" nil)

The tag here should be "Disable input history" or something like that.

> +                 file)
> +  :version "30.1")
> +
>  (defvaralias 'inferior-emacs-lisp-mode-hook 'ielm-mode-hook)
>  (defcustom ielm-mode-hook nil
>    "Hooks to be run when IELM (`inferior-emacs-lisp-mode') is started."
> @@ -503,6 +510,17 @@ ielm--expand-ellipsis
>      (funcall pp-default-function beg end)
>      end))
>
> +;;; Input history
> +
> +(defvar ielm--exit

This should read

    (defvar ielm--exit nil

or the below docstring will instead be its value.

> +  "Function to call when Emacs is killed.")
> +
> +(defun ielm--input-history-writer (buf)
> +  "Return a function writing IELM input history to BUF."
> +  (lambda ()
> +    (with-current-buffer buf
> +      (comint-write-input-ring))))
> +
>  ;;; Major mode
>
>  (define-derived-mode inferior-emacs-lisp-mode comint-mode "IELM"
> @@ -605,6 +623,17 @@ inferior-emacs-lisp-mode
>              #'ielm-indirect-setup-hook 'append t)
>    (setq comint-indirect-setup-function #'emacs-lisp-mode)
>
> +  ;; Input history
> +  (setq-local comint-input-ring-file-name ielm-history-file-name)
> +  (setq-local ielm--exit (ielm--input-history-writer (current-buffer)))
> +  (setq-local kill-buffer-hook
> +              (lambda ()
> +                (funcall ielm--exit)
> +                (remove-hook 'kill-emacs-hook ielm--exit)))
> +  (unless noninteractive
> +    (add-hook 'kill-emacs-hook ielm--exit))
> +  (comint-read-input-ring t)

There are some complications here:

You can get more than one IELM buffer using

    M-x ielm RET
    M-x rename-buffer RET foo RET
    M-x ielm RET

What happens if there is more than one IELM buffer?  It will save only
the history from the last one, or something like that?

Perhaps the kill-emacs-hook should look for all buffers that are using
`ielm-mode' and save the history from all of them?

> +
>    ;; A dummy process to keep comint happy. It will never get any input
>    (unless (comint-check-proc (current-buffer))
>      ;; Was cat, but on non-Unix platforms that might not exist, so
> --
> 2.39.2
>
>
> -- Simen

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi Stefan, thanks for your feedback. An updated patch is attached.

Stefan Kangas <stefankangas@gmail.com> writes:

> Simen Heggestøyl <simenheg@runbox.com> writes:
>
>> +---
>> +*** IELM now remembers input history between sessions.
>> +
>> +---
>> +*** New variable 'ielm-history-file-name'.
>> +If non-nil, name of the file to read/write IELM input history.  Set to
>> +nil to revert IELM to the old behavior of not remembering input
>> +history between sessions.
>
> I would probably make this into one entry, like so:
>
> *** IELM now remembers input history between sessions.
> The new user option 'ielm-history-file-name' is the name of the file
> where IELM input history will be saved.  Customize it to nil to revert
> to the old behavior of not remembering input history between sessions.

Aight, changed!

>> +(defcustom ielm-history-file-name
>> +  (locate-user-emacs-file "ielm-history.eld")
>> +  "If non-nil, name of the file to read/write IELM input history."
>> +  :type '(choice (const :tag "nil" nil)
>
> The tag here should be "Disable input history" or something like that.

Ok. Should it be updated for `comint-input-ring-file-name' too then (I
copied it from there)?

> This should read
>
>     (defvar ielm--exit nil
>
> or the below docstring will instead be its value.

Of course, thanks.

>> +  "Function to call when Emacs is killed.")
>> +
>> +(defun ielm--input-history-writer (buf)
>> +  "Return a function writing IELM input history to BUF."
>> +  (lambda ()
>> +    (with-current-buffer buf
>> +      (comint-write-input-ring))))
>> +
>>  ;;; Major mode
>>
>>  (define-derived-mode inferior-emacs-lisp-mode comint-mode "IELM"
>> @@ -605,6 +623,17 @@ inferior-emacs-lisp-mode
>>              #'ielm-indirect-setup-hook 'append t)
>>    (setq comint-indirect-setup-function #'emacs-lisp-mode)
>>
>> +  ;; Input history
>> +  (setq-local comint-input-ring-file-name ielm-history-file-name)
>> +  (setq-local ielm--exit (ielm--input-history-writer (current-buffer)))
>> +  (setq-local kill-buffer-hook
>> +              (lambda ()
>> +                (funcall ielm--exit)
>> +                (remove-hook 'kill-emacs-hook ielm--exit)))
>> +  (unless noninteractive
>> +    (add-hook 'kill-emacs-hook ielm--exit))
>> +  (comint-read-input-ring t)
>
> There are some complications here:
>
> You can get more than one IELM buffer using
>
>     M-x ielm RET
>     M-x rename-buffer RET foo RET
>     M-x ielm RET
>
> What happens if there is more than one IELM buffer?  It will save only
> the history from the last one, or something like that?

Yes. Though I think that's kind of expected; that's how Eshell for
instance also works.

> Perhaps the kill-emacs-hook should look for all buffers that are using
> `ielm-mode' and save the history from all of them?

It would also need to be handled in the kill-buffer hooks I guess,
updating the remaining buffers' histories when one buffer is
killed. Sounds kind of messy though, I think I'd prefer the more simple
approach suggested here.

-- Simen


[-- Attachment #2: 0001-Add-support-for-reading-writing-IELM-input-history.patch --]
[-- Type: text/x-diff, Size: 3365 bytes --]

From fe91c4f3aaefc109135c48ba26f8f21e87db465e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
Date: Tue, 16 Jan 2024 08:21:41 +0100
Subject: [PATCH] Add support for reading/writing IELM input history

* lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
reading/writing input history.
(ielm--history-file-name): Name of the file to read/write IELM input
history.
(ielm--exit): Holds a function to call when Emacs is killed to write
out the input history.
(ielm--input-history-writer): Helper function for writing the IELM
input history out to file.
---
 etc/NEWS     |  8 ++++++++
 lisp/ielm.el | 29 +++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index ee7462cb2aa..2c98a805582 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1328,6 +1328,14 @@ characters, such as ½ (U+00BD VULGAR FRACTION ONE HALF), are also
 recognized as rational fractions.  They have been since 2004, but it
 looks like it was never mentioned in the NEWS, or even the manual.
 
+** IELM
+
+---
+*** IELM now remembers input history between sessions.
+The new user option 'ielm-history-file-name' is the name of the file
+where IELM input history will be saved.  Customize it to nil to revert
+to the old behavior of not remembering input history between sessions.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/ielm.el b/lisp/ielm.el
index 777aebb70cf..e583e0fe32c 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -110,6 +110,13 @@ ielm-dynamic-multiline-inputs
 such as `edebug-defun' to work with such inputs."
   :type 'boolean)
 
+(defcustom ielm-history-file-name
+  (locate-user-emacs-file "ielm-history.eld")
+  "If non-nil, name of the file to read/write IELM input history."
+  :type '(choice (const :tag "Disable input history" nil)
+                 file)
+  :version "30.1")
+
 (defvaralias 'inferior-emacs-lisp-mode-hook 'ielm-mode-hook)
 (defcustom ielm-mode-hook nil
   "Hooks to be run when IELM (`inferior-emacs-lisp-mode') is started."
@@ -503,6 +510,17 @@ ielm--expand-ellipsis
     (funcall pp-default-function beg end)
     end))
 
+;;; Input history
+
+(defvar ielm--exit nil
+  "Function to call when Emacs is killed.")
+
+(defun ielm--input-history-writer (buf)
+  "Return a function writing IELM input history to BUF."
+  (lambda ()
+    (with-current-buffer buf
+      (comint-write-input-ring))))
+
 ;;; Major mode
 
 (define-derived-mode inferior-emacs-lisp-mode comint-mode "IELM"
@@ -605,6 +623,17 @@ inferior-emacs-lisp-mode
             #'ielm-indirect-setup-hook 'append t)
   (setq comint-indirect-setup-function #'emacs-lisp-mode)
 
+  ;; Input history
+  (setq-local comint-input-ring-file-name ielm-history-file-name)
+  (setq-local ielm--exit (ielm--input-history-writer (current-buffer)))
+  (setq-local kill-buffer-hook
+              (lambda ()
+                (funcall ielm--exit)
+                (remove-hook 'kill-emacs-hook ielm--exit)))
+  (unless noninteractive
+    (add-hook 'kill-emacs-hook ielm--exit))
+  (comint-read-input-ring t)
+
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
     ;; Was cat, but on non-Unix platforms that might not exist, so
-- 
2.39.2

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Stefan Kangas]

Simen Heggestøyl <simenheg@runbox.com> writes:

> Hi Stefan, thanks for your feedback. An updated patch is attached.

LGTM, please install.  Feel free to handle the minor comments below as
you think is best, or disregard them.

Thanks!

>>> +(defcustom ielm-history-file-name
>>> +  (locate-user-emacs-file "ielm-history.eld")
>>> +  "If non-nil, name of the file to read/write IELM input history."
>>> +  :type '(choice (const :tag "nil" nil)
>>
>> The tag here should be "Disable input history" or something like that.
>
> Ok. Should it be updated for `comint-input-ring-file-name' too then (I
> copied it from there)?

Yes, that could be improved too.

> From fe91c4f3aaefc109135c48ba26f8f21e87db465e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
> Date: Tue, 16 Jan 2024 08:21:41 +0100
> Subject: [PATCH] Add support for reading/writing IELM input history
>
> * lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
> reading/writing input history.

How about:

    "Add support for saving input history to a file"

> (ielm--history-file-name): Name of the file to read/write IELM input
> history.

Maybe:

    "New variable indicating IELM input history file.

> +(defcustom ielm-history-file-name
> +  (locate-user-emacs-file "ielm-history.eld")
> +  "If non-nil, name of the file to read/write IELM input history."
> +  :type '(choice (const :tag "Disable input history" nil)
> +                 file)
> +  :version "30.1")

Maybe `file' should have a tag too?

bug#67000: 30.0.50; [PATCH] Add support for reading/writing IELM input history

[Simen Heggestøyl via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> LGTM, please install.

Done! Thanks again for reviewing.

>>>> +(defcustom ielm-history-file-name
>>>> +  (locate-user-emacs-file "ielm-history.eld")
>>>> +  "If non-nil, name of the file to read/write IELM input history."
>>>> +  :type '(choice (const :tag "nil" nil)
>>>
>>> The tag here should be "Disable input history" or something like that.
>>
>> Ok. Should it be updated for `comint-input-ring-file-name' too then (I
>> copied it from there)?
>
> Yes, that could be improved too.

Done.

>> From fe91c4f3aaefc109135c48ba26f8f21e87db465e Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@runbox.com>
>> Date: Tue, 16 Jan 2024 08:21:41 +0100
>> Subject: [PATCH] Add support for reading/writing IELM input history
>>
>> * lisp/ielm.el (inferior-emacs-lisp-mode): Add support for
>> reading/writing input history.
>
> How about:
>
>     "Add support for saving input history to a file"

Changed.

>> (ielm--history-file-name): Name of the file to read/write IELM input
>> history.
>
> Maybe:
>
>     "New variable indicating IELM input history file.

Changed.

>> +(defcustom ielm-history-file-name
>> +  (locate-user-emacs-file "ielm-history.eld")
>> +  "If non-nil, name of the file to read/write IELM input history."
>> +  :type '(choice (const :tag "Disable input history" nil)
>> +                 file)
>> +  :version "30.1")
>
> Maybe `file' should have a tag too?

The label in the Custom menu says "File", which seems fine to me? Though
I never use Custom, so I'm not sure what's customary here. I installed
the above without changes, but please feel free to change it.

-- Simen