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

* Re: ielm automatic saving of history -- bug 67000
  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
                     ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-13  6:06 UTC (permalink / raw)
  To: Madhu, Simen Heggestøyl; +Cc: emacs-devel

> From: Madhu <enometh@meer.net>
> Date: Sun, 13 Oct 2024 09:47:25 +0530
> 
> 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)

I think ielm--input-history-writer should bind coding-system-for-write
to utf-8-emacs-unix around writing to the file, and it should add a
file-locals section stating the encoding (for reading the history
back).

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

If you do that, comint-write-input-ring is supposed to do nothing and
return immediately.  Does that not happen?

However, it sounds safer to avoid adding the hooks in that case as
well.

> 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

How can this happen? can you show a recipe for this?

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

Simen, any comments or suggestions?  This feature is new in Emacs 30,
so it isn't too late to improve it before we release Emacs 30.1.

Thanks.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-13  6:06 ` Eli Zaretskii
@ 2024-10-13  9:49   ` Madhu
  2024-10-14  6:23   ` Augusto Stoffel
  2024-10-16 17:02   ` Simen Heggestøyl
  2 siblings, 0 replies; 17+ messages in thread
From: Madhu @ 2024-10-13  9:49 UTC (permalink / raw)
  To: eliz; +Cc: simenheg, emacs-devel

Hello,

*  Eli Zaretskii <eliz@gnu.org> <86v7xwtu3i.fsf@gnu.org>
Wrote on Sun, 13 Oct 2024 09:06:25 +0300
>> 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.
>
> If you do that, comint-write-input-ring is supposed to do nothing and
> return immediately.  Does that not happen?

It still interferes with any user customization, (e.g. the
my-ielm-mode-hook in the file attached upthread)

>> The "worst" problem happens when ielm buffer exits without running
>> kill-buffer-hook
>
> How can this happen? can you show a recipe for this?

(let (kill-buffer-hook (executing-kbd-macro 'kludge) buffer-file-name kill-buffer-query-functions) (kill-buffer "*ielm*")) -- used as outlined in the quoted portion below, to opt out of saving history for "this" particular ielm session.

I agree it is outside the design-scope of the current implementation,
but it does still hurt, -- Regards, Madhu

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  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-16 17:02   ` Simen Heggestøyl
  2 siblings, 1 reply; 17+ messages in thread
From: Augusto Stoffel @ 2024-10-14  6:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Madhu, Simen Heggestøyl, emacs-devel

On Sun, 13 Oct 2024 at 09:06, Eli Zaretskii wrote:

>> 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.
>
> Simen, any comments or suggestions?  This feature is new in Emacs 30,
> so it isn't too late to improve it before we release Emacs 30.1.

I have a different suggestion.  I think IELM should use savehist-mode to
preserve its history.

When loading ielm.el:

    (cl-pushnew 'ielm-history savehist-minibuffer-history-variables)

When starting an IELM buffer:

    (setq comint-input-ring (make-ring comint-input-ring-size))
    (dolist (cmd (take comint-input-ring-size ielm-history))
      (ring-insert-at-beginning comint-input-ring cmd))

After each evaluation:

    (add-to-history 'ielm-history code comint-input-ring-size)

I use this approach in my dREPL package and it works nicely.  One
advantage (in my opinion) is that there is new configuration variable to
discover and set; IELM history would be preserved if and only if the
user enabled savehist-mode.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-14  6:23   ` Augusto Stoffel
@ 2024-10-14 14:05     ` Eli Zaretskii
  2024-10-15  4:46       ` Madhu
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-14 14:05 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: enometh, simenheg, emacs-devel

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Madhu <enometh@meer.net>,  Simen Heggestøyl
>  <simenheg@runbox.com>,
>   emacs-devel@gnu.org
> Date: Mon, 14 Oct 2024 08:23:09 +0200
> 
> I have a different suggestion.  I think IELM should use savehist-mode to
> preserve its history.
> 
> When loading ielm.el:
> 
>     (cl-pushnew 'ielm-history savehist-minibuffer-history-variables)
> 
> When starting an IELM buffer:
> 
>     (setq comint-input-ring (make-ring comint-input-ring-size))
>     (dolist (cmd (take comint-input-ring-size ielm-history))
>       (ring-insert-at-beginning comint-input-ring cmd))
> 
> After each evaluation:
> 
>     (add-to-history 'ielm-history code comint-input-ring-size)

This addresses only some of the issues brought up by Madhu.  For
example, the main bug, the one with encoding the saved history, is not
addressed, AFAIU.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-14 14:05     ` Eli Zaretskii
@ 2024-10-15  4:46       ` Madhu
  2024-10-15 12:18         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-15  4:46 UTC (permalink / raw)
  To: eliz; +Cc: arstoffel, simenheg, emacs-devel


*  Eli Zaretskii <eliz@gnu.org> <86r08isrtg.fsf@gnu.org>
Wrote on Mon, 14 Oct 2024 17:05:31 +0300
>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Cc: Madhu <enometh@meer.net>,  Simen Heggestøyl
>>  <simenheg@runbox.com>,
>>   emacs-devel@gnu.org
>> Date: Mon, 14 Oct 2024 08:23:09 +0200
>> I have a different suggestion.  I think IELM should use savehist-mode to
>> preserve its history.
>>
>> When loading ielm.el:
>>
>>     (cl-pushnew 'ielm-history savehist-minibuffer-history-variables)
>>
>> When starting an IELM buffer:
>>
>>     (setq comint-input-ring (make-ring comint-input-ring-size))
>>     (dolist (cmd (take comint-input-ring-size ielm-history))
>>       (ring-insert-at-beginning comint-input-ring cmd))
>>
>> After each evaluation:
>>
>>     (add-to-history 'ielm-history code comint-input-ring-size)
>
> This addresses only some of the issues brought up by Madhu.  For
> example, the main bug, the one with encoding the saved history, is not
> addressed, AFAIU.

I think that part is covered. if you see the top of ~/.emacs.d/history
the coding system is specified, the savehist-coding-system mechanism
takes care of this to you.  But I haven't tried this idea out yet (to
see the interactions with comint).



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-15  4:46       ` Madhu
@ 2024-10-15 12:18         ` Eli Zaretskii
  2024-10-15 17:30           ` Madhu
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-15 12:18 UTC (permalink / raw)
  To: Madhu; +Cc: arstoffel, simenheg, emacs-devel

> Date: Tue, 15 Oct 2024 10:16:33 +0530 (IST)
> Cc: arstoffel@gmail.com, simenheg@runbox.com, emacs-devel@gnu.org
> From: Madhu <enometh@meer.net>
> 
> 
> *  Eli Zaretskii <eliz@gnu.org> <86r08isrtg.fsf@gnu.org>
> Wrote on Mon, 14 Oct 2024 17:05:31 +0300
> >> From: Augusto Stoffel <arstoffel@gmail.com>
> >> Cc: Madhu <enometh@meer.net>,  Simen Heggestøyl
> >>  <simenheg@runbox.com>,
> >>   emacs-devel@gnu.org
> >> Date: Mon, 14 Oct 2024 08:23:09 +0200
> >> I have a different suggestion.  I think IELM should use savehist-mode to
> >> preserve its history.
> >>
> >> When loading ielm.el:
> >>
> >>     (cl-pushnew 'ielm-history savehist-minibuffer-history-variables)
> >>
> >> When starting an IELM buffer:
> >>
> >>     (setq comint-input-ring (make-ring comint-input-ring-size))
> >>     (dolist (cmd (take comint-input-ring-size ielm-history))
> >>       (ring-insert-at-beginning comint-input-ring cmd))
> >>
> >> After each evaluation:
> >>
> >>     (add-to-history 'ielm-history code comint-input-ring-size)
> >
> > This addresses only some of the issues brought up by Madhu.  For
> > example, the main bug, the one with encoding the saved history, is not
> > addressed, AFAIU.
> 
> I think that part is covered. if you see the top of ~/.emacs.d/history
> the coding system is specified, the savehist-coding-system mechanism
> takes care of this to you.  But I haven't tried this idea out yet (to
> see the interactions with comint).

The problem, AFAIU, is not the specification of the encoding when the
file is written, the problem is to select the right encoding to begin
with.  utf-8-emacs-unix is how characters are represented in Emacs
internally, so it by definition can encode any character in the
history; this is not true for any other encoding.  I believe this
issue was what prevented you from existing Emacs, the issue you
described in your original post.  Apologies if I misunderstood.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-15 12:18         ` Eli Zaretskii
@ 2024-10-15 17:30           ` Madhu
  2024-10-15 18:23             ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-15 17:30 UTC (permalink / raw)
  To: eliz; +Cc: arstoffel, simenheg, emacs-devel

*  Eli Zaretskii <eliz@gnu.org> <864j5dsgot.fsf@gnu.org>
Wrote on Tue, 15 Oct 2024 15:18:10 +0300
>> Date: Tue, 15 Oct 2024 10:16:33 +0530 (IST)
>> From: Madhu <enometh@meer.net>
[re:  savehist for ielm history]

>> I think that part is covered. if you see the top of ~/.emacs.d/history
>> the coding system is specified, the savehist-coding-system mechanism
>> takes care of this to you.  But I haven't tried this idea out yet (to
>> see the interactions with comint).
>
> The problem, AFAIU, is not the specification of the encoding when the
> file is written, the problem is to select the right encoding to begin
> with.  utf-8-emacs-unix is how characters are represented in Emacs
> internally, so it by definition can encode any character in the
> history; this is not true for any other encoding.  I believe this
> issue was what prevented you from existing Emacs, the issue you
> described in your original post.  Apologies if I misunderstood.


The point I wanted to make was that savehist-save binds
coding-system-for-write (to 'utf-8-unix) and this seems to work
without throwing a "coding system error".

my ~/.ielm-history (which gets read into comint-input-ring) has this
line (but with raw characters)

```
(rfc2047-encode-string "foo \303\200 bar")
```

(funcall ielm--exit) in the ielm buffer causes the coding system warning:

"These default coding systems were tried to encode the
following[...].. utf-8-unix"

However

```
(let ((coding-system-for-write 'utf-8-unix)) (funcall ielm--exit))
```

saves the history according to the code in ielm--exit without the
coding system error.

but coding-system-for-write is still 'utf-8-unix and not
'utf-8-emacs-unix so I think I still need an explanation, please.

(Also I think I forgot to mention the code in ielm-mode presently sets
kill-buffer-hook to a function rather than adding the function to the
hook variable)



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-15 17:30           ` Madhu
@ 2024-10-15 18:23             ` Eli Zaretskii
  2024-10-16  4:25               ` Madhu
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-15 18:23 UTC (permalink / raw)
  To: Madhu; +Cc: arstoffel, simenheg, emacs-devel

> Date: Tue, 15 Oct 2024 23:00:11 +0530 (IST)
> Cc: arstoffel@gmail.com, simenheg@runbox.com, emacs-devel@gnu.org
> From: Madhu <enometh@meer.net>
> 
> *  Eli Zaretskii <eliz@gnu.org> <864j5dsgot.fsf@gnu.org>
> Wrote on Tue, 15 Oct 2024 15:18:10 +0300
> >> Date: Tue, 15 Oct 2024 10:16:33 +0530 (IST)
> >> From: Madhu <enometh@meer.net>
> [re:  savehist for ielm history]
> 
> > The problem, AFAIU, is not the specification of the encoding when the
> > file is written, the problem is to select the right encoding to begin
> > with.  utf-8-emacs-unix is how characters are represented in Emacs
> > internally, so it by definition can encode any character in the
> > history; this is not true for any other encoding.  I believe this
> > issue was what prevented you from existing Emacs, the issue you
> > described in your original post.  Apologies if I misunderstood.
> 
> 
> The point I wanted to make was that savehist-save binds
> coding-system-for-write (to 'utf-8-unix) and this seems to work
> without throwing a "coding system error".

utf-8-unix cannot save some raw bytes, which could happen in the
history of interactive commands such as IELM and other clients of
comint.

> my ~/.ielm-history (which gets read into comint-input-ring) has this
> line (but with raw characters)
> 
> ```
> (rfc2047-encode-string "foo \303\200 bar")
> ```
> 
> (funcall ielm--exit) in the ielm buffer causes the coding system warning:
> 
> "These default coding systems were tried to encode the
> following[...].. utf-8-unix"
> 
> However
> 
> ```
> (let ((coding-system-for-write 'utf-8-unix)) (funcall ielm--exit))
> ```
> 
> saves the history according to the code in ielm--exit without the
> coding system error.

And loses some of those bytes as part of that.  This is exactly why I
suggest to use utf-8-emacs-unix.

> but coding-system-for-write is still 'utf-8-unix and not
> 'utf-8-emacs-unix so I think I still need an explanation, please.

I'm not sure what to explain.  utf-8-unix will not correctly write out
raw bytes and some other characters outside of the Unicode range,
whereas utf-8-emacs-unix will correctly write all of them, because it
uses the same encoding as the internal representation of characters
within Emacs.

If this is still not clear, please ask specific questions.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-15 18:23             ` Eli Zaretskii
@ 2024-10-16  4:25               ` Madhu
  2024-10-16  6:04                 ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-16  4:25 UTC (permalink / raw)
  To: eliz; +Cc: emacs-devel

*  Eli Zaretskii <eliz@gnu.org> <86jze9ql81.fsf@gnu.org>
Wrote on Tue, 15 Oct 2024 21:23:10 +0300

>> my ~/.ielm-history (which gets read into comint-input-ring) has this
>> line (but with raw characters)
>>
>> ```
>> (rfc2047-encode-string "foo \303\200 bar")
>> ```
>>
>> (funcall ielm--exit) in the ielm buffer causes the coding system warning:
>>
>> "These default coding systems were tried to encode the
>> following[...].. utf-8-unix"
>>
>> However
>>
>> ```
>> (let ((coding-system-for-write 'utf-8-unix)) (funcall ielm--exit))
>> ```
>>
>> saves the history according to the code in ielm--exit without the
>> coding system error.
>
> And loses some of those bytes as part of that.  This is exactly why I
> suggest to use utf-8-emacs-unix.

In this case there was no loss of bytes. Apparently using utf-8-unix
is sufficient to encode the contents of the buffer beings saved
without any loss of data. The file written is identical to the file
which was read.

(let ((comint-input-ring-file-name "/dev/shm/f") (coding-system-for-write 'utf-8-unix)) (comint-write-input-ring))

>> but coding-system-for-write is still 'utf-8-unix and not
>> 'utf-8-emacs-unix so I think I still need an explanation, please.
>
> I'm not sure what to explain.  utf-8-unix will not correctly write out
> raw bytes and some other characters outside of the Unicode range,
> whereas utf-8-emacs-unix will correctly write all of them, because it
> uses the same encoding as the internal representation of characters
> within Emacs.
>
> If this is still not clear, please ask specific questions.

I think the question I wanted to ask was: does specifying
coding-system-for-write (to any non-nil acceptable value)
unconditionally suppress the "coding system warning" error?  from your
response above I think the answer is: "yes, but it may clobber it".

The second issue is that the coding system warning which is printed
when emacs prompts for a suitable coding system isn't entire accurate.
it says the default coding system which was tried, to encode ".."  is
utf-8-unix and cannot encode these characters etc. but if it is
sufficient, and if i type utf-8-unix into the prompt, the file is
encoded and saved.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-16  4:25               ` Madhu
@ 2024-10-16  6:04                 ` Eli Zaretskii
  2024-10-16  9:03                   ` Madhu
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-16  6:04 UTC (permalink / raw)
  To: Madhu; +Cc: emacs-devel

> Date: Wed, 16 Oct 2024 09:55:42 +0530 (IST)
> Cc: emacs-devel@gnu.org
> From: Madhu <enometh@meer.net>
> 
> >> (let ((coding-system-for-write 'utf-8-unix)) (funcall ielm--exit))
> >> ```
> >>
> >> saves the history according to the code in ielm--exit without the
> >> coding system error.
> >
> > And loses some of those bytes as part of that.  This is exactly why I
> > suggest to use utf-8-emacs-unix.
> 
> In this case there was no loss of bytes.

There was, trust me.

> Apparently using utf-8-unix
> is sufficient to encode the contents of the buffer beings saved
> without any loss of data.

It is not.  It can only safely encode characters that are inside the
Unicode codepoint space, and raw bytes are outside that range.

> The file written is identical to the file which was read.

I don't understand what you mean.  AFAIU, IELM history records what
you typed, so what file was read here?

> I think the question I wanted to ask was: does specifying
> coding-system-for-write (to any non-nil acceptable value)
> unconditionally suppress the "coding system warning" error?

No.  It is only sufficient if Emacs has some way of dealing with the
characters which prompted the coding system warning in the first
place.  That is not always the case.

> from your
> response above I think the answer is: "yes, but it may clobber it".

It might clobber them, but in some cases it will actually be unable to
do even that.

> The second issue is that the coding system warning which is printed
> when emacs prompts for a suitable coding system isn't entire accurate.
> it says the default coding system which was tried, to encode ".."  is
> utf-8-unix and cannot encode these characters etc. but if it is
> sufficient, and if i type utf-8-unix into the prompt, the file is
> encoded and saved.

See above: it means cannot encode safely.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-16  6:04                 ` Eli Zaretskii
@ 2024-10-16  9:03                   ` Madhu
  2024-10-16 18:36                     ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-16  9:03 UTC (permalink / raw)
  To: eliz; +Cc: emacs-devel

*  Eli Zaretskii <eliz@gnu.org> <865xpsr3b0.fsf@gnu.org>
Wrote on Wed, 16 Oct 2024 09:04:51 +0300
>> Apparently using utf-8-unix
>> is sufficient to encode the contents of the buffer beings saved
>> without any loss of data.
>
> It is not.  It can only safely encode characters that are inside the
> Unicode codepoint space, and raw bytes are outside that range.
>
>> The file written is identical to the file which was read.
>
> I don't understand what you mean.  AFAIU, IELM history records what
> you typed, so what file was read here?

the ielm-history file from which the comint-input-ring is initalized

If I copy my ~/.ielm-history.el to /dev/shm/2.txt.binary and run


```
(let ((comint-input-ring-file-name "/dev/shm/2.txt.binary")) (comint-read-input-ring))
```

it initializes the input-ring from the history. emacs apparently reads this in raw-text-unix.


```
(let ((comint-input-ring-file-name "/dev/shm/2.txt.out")) (comint-write-output-ring))
```

Here I am prompted with the coding system warning . If i go ahead and qchoose
utf-8-unix, it goes ahead and saves it. I can compare 2.txt.binary and
2.txt.out to see that utf-8-unix has not resulted in any data loss.
Hence all the claims in my previous message.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-13  6:06 ` Eli Zaretskii
  2024-10-13  9:49   ` Madhu
  2024-10-14  6:23   ` Augusto Stoffel
@ 2024-10-16 17:02   ` Simen Heggestøyl
  2024-10-17  2:04     ` Madhu
  2 siblings, 1 reply; 17+ messages in thread
From: Simen Heggestøyl @ 2024-10-16 17:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Madhu, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Simen, any comments or suggestions?  This feature is new in Emacs 30,
> so it isn't too late to improve it before we release Emacs 30.1.

Hi, sorry for the late reply.

I don't have time to devote to this issue on the short term (that is,
before the release of Emacs 30.1). However I see that the discussion has
carried on since you sent me this question. It would be good if you find
a way to improve it enough in time to ship with Emacs 30.1, otherwise
reverting my commit is also fine, and I may have the time to help
improve the feature for a later release.

-- Simen



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-16  9:03                   ` Madhu
@ 2024-10-16 18:36                     ` Eli Zaretskii
  2024-10-31 10:42                       ` Madhu
  0 siblings, 1 reply; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-16 18:36 UTC (permalink / raw)
  To: Madhu; +Cc: emacs-devel

> Date: Wed, 16 Oct 2024 14:33:59 +0530 (IST)
> Cc: emacs-devel@gnu.org
> From: Madhu <enometh@meer.net>
> 
> *  Eli Zaretskii <eliz@gnu.org> <865xpsr3b0.fsf@gnu.org>
> >> The file written is identical to the file which was read.
> >
> > I don't understand what you mean.  AFAIU, IELM history records what
> > you typed, so what file was read here?
> 
> the ielm-history file from which the comint-input-ring is initalized

That's not what bothers me.  What bothers me is the way we save the
history of the ielm command the first time, when they are not taken
from a file, but from the user input.

> Here I am prompted with the coding system warning . If i go ahead and qchoose
> utf-8-unix, it goes ahead and saves it. I can compare 2.txt.binary and
> 2.txt.out to see that utf-8-unix has not resulted in any data loss.
> Hence all the claims in my previous message.

The data loss I had in mind is not in this case.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-16 17:02   ` Simen Heggestøyl
@ 2024-10-17  2:04     ` Madhu
  0 siblings, 0 replies; 17+ messages in thread
From: Madhu @ 2024-10-17  2:04 UTC (permalink / raw)
  To: simenheg; +Cc: eliz, emacs-devel

[-- Attachment #1: Type: Text/Plain, Size: 982 bytes --]

*  Simen Heggestøyl <simenheg@runbox.com> <877ca8vv59.fsf@runbox.com>
Wrote on Wed, 16 Oct 2024 19:02:10 +0200
> I don't have time to devote to this issue on the short term (that is,
> before the release of Emacs 30.1). However I see that the discussion has
> carried on since you sent me this question. It would be good if you find
> a way to improve it enough in time to ship with Emacs 30.1, otherwise
> reverting my commit is also fine, and I may have the time to help
> improve the feature for a later release.

I am attaching a patch (not fully tested) where I tried to fix the
most important issues with the history mechanism. Perhaps Eli could
review and commit parts of as he sees fit (maybe dropping the change
to the default value). btw I don't think it is possible to stick file
local variables into the file read by comint-read-input-ring

Or alternatively maybe bug67000 should be reopened and this should be
continued there. -- Best Regards. Madhu


[-- Attachment #2: 0001-fix-some-ielm-history-issues.patch --]
[-- Type: Text/X-Patch, Size: 2950 bytes --]

From fe78a75cc3d322b3b8f2df24d50dca1b1216d894 Mon Sep 17 00:00:00 2001
From: Madhu <enometh@net.meer>
Date: Fri, 11 Oct 2024 21:16:46 +0530
Subject: [PATCH] fix some issues in the persistent ielm history mechanism
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

this fixes some issues in commit 60cff1ac9d21. (bug 67000)

* lisp/elm.el: (ielm-history-file-name): initially
nil. (inferior-emacs-lisp-mode): do not invoke the machinery at all if
ielm-history-file-name is nil. use add-hook on kill-buffer-hook instead
of replacing kill-buffer-hook. bind coding-system-for-read around call
to comint-read-input-ring.  (ielm-input-history-writer): bind
coding-system-on-write around call to comint-write-input-ring.

---
 lisp/ielm.el | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/lisp/ielm.el b/lisp/ielm.el
index f62db7510de..000ba48ba68 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -111,7 +111,8 @@ ielm-dynamic-multiline-inputs
   :type 'boolean)
 
 (defcustom ielm-history-file-name
-  (locate-user-emacs-file "ielm-history.eld")
+  (if t nil
+      (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)
@@ -520,7 +521,9 @@ ielm--input-history-writer
   "Return a function writing IELM input history to BUF."
   (lambda ()
     (with-current-buffer buf
-      (comint-write-input-ring))))
+      (let ((coding-system-for-write 'utf-8-emacs-unix))
+        ;; cannot add a file-local section when using comint.
+        (comint-write-input-ring)))))
 
 ;;; Major mode
 
@@ -623,17 +626,19 @@ inferior-emacs-lisp-mode
   (add-hook 'comint-indirect-setup-hook
             #'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
+  (when ielm-history-file-name
+    ;; Input history
+    (setq-local comint-input-ring-file-name ielm-history-file-name)
+    (setq-local ielm--exit (ielm--input-history-writer (current-buffer)))
+    (add-hook '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)
+                (remove-hook 'kill-emacs-hook ielm--exit))
+              nil t)
+    (unless noninteractive
+      (add-hook 'kill-emacs-hook ielm--exit))
+    (let ((coding-system-for-read 'utf-8-unix))
+      (comint-read-input-ring t)))
 
   ;; A dummy process to keep comint happy. It will never get any input
   (unless (comint-check-proc (current-buffer))
-- 
2.46.0.27.gfa3b914457


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-16 18:36                     ` Eli Zaretskii
@ 2024-10-31 10:42                       ` Madhu
  2024-10-31 11:13                         ` Eli Zaretskii
  0 siblings, 1 reply; 17+ messages in thread
From: Madhu @ 2024-10-31 10:42 UTC (permalink / raw)
  To: eliz; +Cc: emacs-devel

*  Eli Zaretskii  <86jze7q4io.fsf@gnu.org>
Wrote on Wed, 16 Oct 2024 21:36:15 +0300
>> the ielm-history file from which the comint-input-ring is initalized
>
> That's not what bothers me.  What bothers me is the way we save the
> history of the ielm command the first time, when they are not taken
> from a file, but from the user input.
>
>> Here I am prompted with the coding system warning . If i go ahead
>> and qchoose utf-8-unix, it goes ahead and saves it. I can compare
>> 2.txt.binary and 2.txt.out to see that utf-8-unix has not resulted
>> in any data loss.  Hence all the claims in my previous message.
>
> The data loss I had in mind is not in this case.

I still think there is some fishy behaviour when raw-text-unix is used
by as the initial (unspecified) encoding.  The message which the
prompt presents (that utf-8-unix cannot be used to save etc.) is
surely incorrect, since it goes ahead and saves it anyway as
utf-8-unix.

BTW did you have a chance to consider the patch I mailed on
oct-17-2024? -- Regards, madhu



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: ielm automatic saving of history -- bug 67000
  2024-10-31 10:42                       ` Madhu
@ 2024-10-31 11:13                         ` Eli Zaretskii
  0 siblings, 0 replies; 17+ messages in thread
From: Eli Zaretskii @ 2024-10-31 11:13 UTC (permalink / raw)
  To: Madhu; +Cc: emacs-devel

> Date: Thu, 31 Oct 2024 16:12:05 +0530 (IST)
> Cc: emacs-devel@gnu.org
> From: Madhu <enometh@meer.net>
> 
> *  Eli Zaretskii  <86jze7q4io.fsf@gnu.org>
> Wrote on Wed, 16 Oct 2024 21:36:15 +0300
> >> the ielm-history file from which the comint-input-ring is initalized
> >
> > That's not what bothers me.  What bothers me is the way we save the
> > history of the ielm command the first time, when they are not taken
> > from a file, but from the user input.
> >
> >> Here I am prompted with the coding system warning . If i go ahead
> >> and qchoose utf-8-unix, it goes ahead and saves it. I can compare
> >> 2.txt.binary and 2.txt.out to see that utf-8-unix has not resulted
> >> in any data loss.  Hence all the claims in my previous message.
> >
> > The data loss I had in mind is not in this case.
> 
> I still think there is some fishy behaviour when raw-text-unix is used
> by as the initial (unspecified) encoding.  The message which the
> prompt presents (that utf-8-unix cannot be used to save etc.) is
> surely incorrect, since it goes ahead and saves it anyway as
> utf-8-unix.

The explanation presented when Emacs requests a safe encoding was
worded in a way that could be interpreted to mean the text cannot
possibly be encoded bu utf-8.  The reality is subtly different: utf-8
cannot encode that _safely_, but it can encode it with some data loss
if forced to do so.  I have modified the wording to say "safely
encode", to make that part more explicit.

> BTW did you have a chance to consider the patch I mailed on
> oct-17-2024? -- Regards, madhu

No, I hoped Simen will get to that at some point.



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