unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* ielm automatic saving of history -- bug 67000
@ 2024-10-13  4:17 Madhu
  2024-10-13  6:06 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-13  4:17 UTC (permalink / raw)
  To: emacs-devel

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

* commit 60cff1ac9d216e5abcb350ea5e623ab0b377c131
|Author:     Simen Heggestøyl <simenheg@runbox.com>
|AuthorDate: Tue Jan 16 08:21:41 2024 +0100
|Commit:     Simen Heggestøyl <simenheg@runbox.com>
|CommitDate: Thu Feb 15 08:46:28 2024 +0100
|
|    Add support for reading/writing IELM input history (bug#67000)
|
|    * lisp/ielm.el (inferior-emacs-lisp-mode): Add support for saving input
|    history to a file.
|    (ielm--history-file-name): New variable indicating IELM input history
|    file.
|    (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.


I see several problems with this implementation (based on opaque
closures), the worst of which is that I am unable to quit emacs, because
a closure (which tries to save the contents of a killed-buffer, is in
kill-emacs-hook, and it cannot get the coding system right)


The implementation roughly does:
```
      (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)
```

2. The way to opt out of this is to set ielm-history-file-name to nil in
user customization.  In that case this code path should be avoided
altogether.

ielm--exit is a buffer local variable which gets set to
    ```
      (lambda ()
        (with-current-buffer buf
          (comint-write-input-ring)))
    ```

and pushed onto kill-emacs-hook. Another closure gets pushed onto
kill-buffer-hook, which removes the elt in kill-emacs-hook when it
runs.

The "worst" problem happens when ielm buffer exits without running
kill-buffer-hook ( -- historically I've used this pattern to avoid
saving history for a particular buffer, this is the way to opt out of
saving the history) but in this case it runs from the kill-emacs-hook,
and that forces me to choose a coding system to save the buffer. There
is no option to opt out.

I don't have any suggestions on how to solve this with this design.

3. The default Comint functions for storing eliisp history is less than
acceptable. it stores lines, while ielm and elisp permit multi-line sexp
forms.

personally I've used code (derived from sly so not probably fsf license)
in ielm-mode-hook to set up merge and save multiline forms using comint,
I'm attaching this file here, for reference.  However the currenlty
installed implementation in (inferior-emacs-lisp-mode) does not play
well with the this: It unconditionally touches comint history variables,
my code also has to set the comint variables to use the comint history
mechanism, and these are picked up with exit hooks.

Maybe others using the facility will have some opinions on this. My own
takeaway is it is not desirable to 1) force history saving mechanisms 2)
use opaque closures for implementing hook functions.



[-- Attachment #2: my-ielm-history.el --]
[-- Type: text/plain, Size: 2935 bytes --]

;;; my-ielm-history.el -- to save ielm history using a hook.  (posted,
;;; madhu 20241013)

;; comint-input-ring-separator and ielm-write-input-wring taken from
;; sly

(defun my-ielm-read-input-ring ()
  (let ((comint-input-ring-separator "####\n"))
    (comint-read-input-ring)))

(defun my-ielm-process-sentinel (proc msg)
  (let ((buffer (process-buffer (get-process proc))))
    (when (buffer-live-p buffer)
      (with-current-buffer buffer
	(my-ielm-write-input-ring))))
  (internal-default-process-sentinel proc msg))

(defun my-ielm-write-input-ring ()
  ;; not enough to make it buffer-local. The binding has to exist when
  ;; the rings is being written to " *Temp Input History*"
  (let* ((comint-input-ring-separator "####\n")
	 ;; To merge the file's history with the current buffer's
	 ;; history, sntart by deep-copying `comint-input-ring' to a
	 ;; separate variable.
	 (current-ring (copy-tree comint-input-ring 'vectors-too))
	 (histcontrol 'erasedups)
	 (index (ring-length current-ring))
	 )
    ;; this sets `comint-input-ring' from the file
    (my-ielm-read-input-ring)
    ;; loop `current-ring', which potentially contains new entries and
    ;; re-add entries to `comint-input-ring', which is now synched
    ;; with the file and will be written to disk.
    (cl-loop for i from (1- index) downto 0
             for item = (ring-ref current-ring i)
             for existing-index = (ring-member comint-input-ring item)
             do (cond ((and existing-index
                            (eq histcontrol 'erasedups))
                       (ring-remove comint-input-ring existing-index)
                       (ring-insert comint-input-ring item))
                      ((and existing-index
                            (not histcontrol))
                       (ring-insert comint-input-ring item))
                      (t
                       (ring-insert comint-input-ring item)))
             unless (ring-member comint-input-ring item)
             do (ring-insert comint-input-ring item))
    ;; Now save `comint-input-ring'
    (let ((coding-system-for-write 'utf-8-unix))
      (comint-write-input-ring))))

;; madhu 060520
(add-hook 'ielm-mode-hook
	  (defun my-ielm-mode-hook ()
	    (set-process-sentinel (get-buffer-process (current-buffer))
				  'my-ielm-process-sentinel)
	    ;;(make-variable-buffer-local 'comint-input-ring-separator)
	    (setq comint-input-ignoredups t
		  comint-input-ring-file-name
		  ;;comint-input-ring-separator "####\n" ;madhu 200131
		  (expand-file-name "~/.ielm-history.el")
		  comint-input-ring-size 1000)
	    ;;(make-local-hook 'kill-buffer-hook)
	    (add-hook 'kill-buffer-hook 'my-ielm-write-input-ring nil t)))

(defun fix-ielm-buffer ()
  "put #### markers between sexps in a ielm history buffer without those"
  (while (not (eobp))
    (forward-sexp)
    (assert (eolp))
    (cond((eolp)
	  (insert "####")
	  (forward-line 1))
	 (t (debug)))))


^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-10-31 11:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-13  4:17 ielm automatic saving of history -- bug 67000 Madhu
2024-10-13  6:06 ` Eli Zaretskii
2024-10-13  9:49   ` Madhu
2024-10-14  6:23   ` Augusto Stoffel
2024-10-14 14:05     ` Eli Zaretskii
2024-10-15  4:46       ` Madhu
2024-10-15 12:18         ` Eli Zaretskii
2024-10-15 17:30           ` Madhu
2024-10-15 18:23             ` Eli Zaretskii
2024-10-16  4:25               ` Madhu
2024-10-16  6:04                 ` Eli Zaretskii
2024-10-16  9:03                   ` Madhu
2024-10-16 18:36                     ` Eli Zaretskii
2024-10-31 10:42                       ` Madhu
2024-10-31 11:13                         ` Eli Zaretskii
2024-10-16 17:02   ` Simen Heggestøyl
2024-10-17  2:04     ` Madhu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).