GNU bug report logs - #74881
31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp files on kill-emacs

Previous Next

Package: emacs;

Reported by: Lin Sun <sunlin7 <at> hotmail.com>

Date: Sun, 15 Dec 2024 06:00:02 UTC

Severity: normal

Tags: patch

Found in version 31.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74881 in the body.
You can then email your comments to 74881 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 15 Dec 2024 06:00:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Lin Sun <sunlin7 <at> hotmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 15 Dec 2024 06:00:03 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp files on
 kill-emacs
Date: Sun, 15 Dec 2024 05:59:40 +0000
[Message part 1 (text/plain, inline)]
The ediff lefts temporary files "/tmp/fineDiff*" when kill emacs during an ediff comparing.

Reproducing steps:

1. echo 0 > /tmp/0; echo 1 > /tmp/1

2. emacs -nw -q -f ediff-files-command /tmp/0 /tmp/1 

on the ediff control buffer, press "n" to move to first difference; there should have two temp files like /tmp/fineDiffA* and /tmp/fineDiffB* were generated by ediff, stay on the ediff buffer (do not press "q" to quit the ediff session). then

3. Press "C-x C-c" or kill-emacs to quit the emacs directly.

Then two temp files /tmp/fineDiff* were left there.

This patch will make sure the temp files be removed when kill-emacs without quitting the ediff session.
[0001-lisp-vc-ediff-util.el-Remove-temp-files-on-kill-emac.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 15 Dec 2024 08:05:03 GMT) Full text and rfc822 format available.

Message #8 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50;
 [PATCH] * lisp/vc/ediff-util.el: Remove temp files on kill-emacs
Date: Sun, 15 Dec 2024 10:04:19 +0200
> From: Lin Sun <sunlin7 <at> hotmail.com>
> Date: Sun, 15 Dec 2024 05:59:40 +0000
> 
> @@ -488,6 +494,7 @@ ediff-setup
>        (if (ediff-buffer-live-p ediff-meta-buffer)
>  	  (ediff-update-meta-buffer
>  	   ediff-meta-buffer nil ediff-meta-session-number))
> +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
>        (run-hooks 'ediff-startup-hook)
>        ) ; eval in control-buffer
>      control-buffer))

This should be carefully programmed to avoid preventing Emacs from
exiting due to some problem.  If ediff-delete-temp-files or one of the
functions it calls can signal an error, it should be wrapped by
condition-case, and if it or one of its callees can try interacting
with the user, we should use kill-emacs-query-functions instead.

Alternatively, we could end the Ediff session when Emacs is killed.

Bottom line: this is a minor cleanup feature, so we should be very
careful not to cause any regressions and problems just because we want
to exit more cleanly.  (On most systems, files in /tmp are routinely
deleted by system's cleanup processes anyway.)

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 15 Dec 2024 17:26:01 GMT) Full text and rfc822 format available.

Message #11 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sun, 15 Dec 2024 17:25:29 +0000
> From: Eli Zaretskii <eliz <at> gnu.org>
> Sent: Sunday, December 15, 2024 12:04 AM
> > From: Lin Sun <sunlin7 <at> hotmail.com>
> > Date: Sun, 15 Dec 2024 05:59:40 +0000
> >
> > @@ -488,6 +494,7 @@ ediff-setup
> >        (if (ediff-buffer-live-p ediff-meta-buffer)
> >          (ediff-update-meta-buffer
> >           ediff-meta-buffer nil ediff-meta-session-number))
> > +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
> >        (run-hooks 'ediff-startup-hook)
> >        ) ; eval in control-buffer
> >      control-buffer))
>
> This should be carefully programmed to avoid preventing Emacs from
> exiting due to some problem.  If ediff-delete-temp-files or one of the
> functions it calls can signal an error, it should be wrapped by
> condition-case, and if it or one of its callees can try interacting
> with the user, we should use kill-emacs-query-functions instead.

The function ediff-delete-temp-files was called at the tail of Ediff  quit routine, 
will be also safe on kill-emacs routine, and can confirm it dose not interactive 
with the user. Actually it deletes the temp files created by Ediff-mode, should 
has full privilege to do its job.

> Alternatively, we could end the Ediff session when Emacs is killed.

It maybe heavy to end the Ediff session if the user choose 
`ediff-setup-windows-plain` as the `ediff-window-setup-function` for a graphic 
frame, then ending a Ediff session will trigger emacs graphic frames layout change 
(Ediff will restore frames layout to the one before its startup).  So here we just try 
clean up the temp files to avoid the heavy works.

> Bottom line: this is a minor cleanup feature, so we should be very
> careful not to cause any regressions and problems just because we want
> to exit more cleanly.  (On most systems, files in /tmp are routinely
> deleted by system's cleanup processes anyway.)

Agree and calling the ediff-delete-temp-files should only for the scenario 
that user kill emacs during an Ediff-session, otherwise it will do nothing. 

Thank you.



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Fri, 27 Dec 2024 05:40:01 GMT) Full text and rfc822 format available.

Message #14 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Fri, 27 Dec 2024 05:38:57 +0000
Hi Eli,

I checked the "ediff-delete-temp-files", it should be safe for executing.  Is it more safe to wrap it with `ignore-errors` ? Like: 

+(defun ediff--delete-temp-files-on-kill-emacs ()
+  "Delete the temp-files associated with the ediff buffers."
+  (dolist (b (buffer-list))
+    (with-current-buffer b
+      (when (eq major-mode 'ediff-mode)
+        (ignore-errors (ediff-delete-temp-files))))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Fri, 27 Dec 2024 08:11:01 GMT) Full text and rfc822 format available.

Message #17 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Fri, 27 Dec 2024 10:10:37 +0200
> From: Lin Sun <sunlin7 <at> hotmail.com>
> CC: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
> Date: Fri, 27 Dec 2024 05:38:57 +0000
> 
> Hi Eli,
> 
> I checked the "ediff-delete-temp-files", it should be safe for executing.  Is it more safe to wrap it with `ignore-errors` ? Like: 
> 
> +(defun ediff--delete-temp-files-on-kill-emacs ()
> +  "Delete the temp-files associated with the ediff buffers."
> +  (dolist (b (buffer-list))
> +    (with-current-buffer b
> +      (when (eq major-mode 'ediff-mode)
> +        (ignore-errors (ediff-delete-temp-files))))))
> 

Yes, probably.  For a good measure, I'd also bind inhibit-interaction
to a non-nil value, to make sure we never ever ask the user anything
inside ediff-delete-temp-files.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Fri, 27 Dec 2024 17:35:01 GMT) Full text and rfc822 format available.

Message #20 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Fri, 27 Dec 2024 17:34:31 +0000
[Message part 1 (text/plain, inline)]
> From: Eli Zaretskii <eliz <at> gnu.org>
> Yes, probably.  For a good measure, I'd also bind inhibit-interaction
> to a non-nil value, to make sure we never ever ask the user anything
> inside ediff-delete-temp-files.

Sure, that's will be more reliable. I had attached the modified patch, please help review again. Thank you !
[Message part 2 (text/html, inline)]
[0001-lisp-vc-ediff-util.el-Remove-temp-files-on-kill-emac.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sat, 28 Dec 2024 07:41:01 GMT) Full text and rfc822 format available.

Message #23 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sat, 28 Dec 2024 09:38:08 +0200
> From: Lin Sun <sunlin7 <at> hotmail.com>
> CC: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
> Date: Fri, 27 Dec 2024 17:34:31 +0000
> 
> > From: Eli Zaretskii <eliz <at> gnu.org>
> > Yes, probably.  For a good measure, I'd also bind inhibit-interaction
> > to a non-nil value, to make sure we never ever ask the user anything
> > inside ediff-delete-temp-files.
> 
> Sure, that's will be more reliable. I had attached the modified patch, please help review again. Thank you !
> 
> From 39b23f1b3f3d58569b7fa4742e8fd24e2cc7071f Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7 <at> hotmail.com>
> Date: Sun, 15 Dec 2024 06:52:17 +0000
> Subject: [PATCH] * lisp/vc/ediff-util.el: Remove temp files on kill-emacs
> 
> ---
>  lisp/vc/ediff-util.el | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
> index 6038f3eae30..e46dc70218b 100644
> --- a/lisp/vc/ediff-util.el
> +++ b/lisp/vc/ediff-util.el
> @@ -213,6 +213,14 @@ ediff-setup-keymap
>    (fset 'ediff-mode-map ediff-mode-map)
>    (run-hooks 'ediff-keymap-setup-hook))
>  
> +(defun ediff--delete-temp-files-on-kill-emacs ()
> +  "Delete the temp-files associated with the ediff buffers."
> +  (let ((inhibit-interaction nil))
                                ^^^
This should be t, not nil.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sat, 28 Dec 2024 08:06:01 GMT) Full text and rfc822 format available.

Message #26 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sat, 28 Dec 2024 08:04:52 +0000
[Message part 1 (text/plain, inline)]
> From: Eli Zaretskii <eliz <at> gnu.org>
> Sent: Friday, December 27, 2024 11:38 PM
> > +  (let ((inhibit-interaction nil))
>                                 ^^^
> This should be t, not nil.

My apologies for posted the testing code (I toggled t/nil to test the behavior on my local).

And I attached a new version of the patch, please help review again. Thank you!
[Message part 2 (text/html, inline)]
[0001-lisp-vc-ediff-util.el-Remove-temp-files-on-kill-emac.patch (text/x-patch, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Sat, 28 Dec 2024 12:42:02 GMT) Full text and rfc822 format available.

Notification sent to Lin Sun <sunlin7 <at> hotmail.com>:
bug acknowledged by developer. (Sat, 28 Dec 2024 12:42:03 GMT) Full text and rfc822 format available.

Message #31 received at 74881-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: 74881-done <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sat, 28 Dec 2024 14:41:07 +0200
> From: Lin Sun <sunlin7 <at> hotmail.com>
> CC: "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
> Date: Sat, 28 Dec 2024 08:04:52 +0000
> 
> > From: Eli Zaretskii <eliz <at> gnu.org>
> > Sent: Friday, December 27, 2024 11:38 PM
> > > +  (let ((inhibit-interaction nil))
> >                                 ^^^
> > This should be t, not nil.
> 
> My apologies for posted the testing code (I toggled t/nil to test the behavior on my local).
> 
> And I attached a new version of the patch, please help review again. Thank you!

Thanks, installed on the master branch, and closing the bug.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sat, 28 Dec 2024 17:41:02 GMT) Full text and rfc822 format available.

Message #34 received at 74881-done <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "74881-done <at> debbugs.gnu.org" <74881-done <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sat, 28 Dec 2024 17:40:35 +0000
[Message part 1 (text/plain, inline)]
> From: Eli Zaretskii <eliz <at> gnu.org>
> Sent: Saturday, December 28, 2024 04:41 AM
>
> Thanks, installed on the master branch, and closing the bug.

Thank you !
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 29 Dec 2024 01:49:02 GMT) Full text and rfc822 format available.

Message #37 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Sun, 29 Dec 2024 02:49:14 +0100
Hi,

I'm a bit late, feel free to ignore may comments if you think it's not
worth the trouble.

Lin Sun <sunlin7 <at> hotmail.com> writes:

> +(defun ediff--delete-temp-files-on-kill-emacs ()
> +  "Delete the temp-files associated with the ediff buffers."
> +  (ignore-errors
> +    (let ((inhibit-interaction t))
> +      (dolist (b (buffer-list))
> +        (with-current-buffer b
> +          (when (eq major-mode 'ediff-mode)
> +            (ediff-delete-temp-files)))))))


I think this is the same as mapping over `ediff-session-registry' which
should hold exactly the list of buffers we want.

Second: Can we move the `ignore-errors' inwards so that an error in one
case doesn't abort the complete loop?


>  ;;; Setup functions
>
> @@ -488,6 +496,7 @@ ediff-setup
>        (if (ediff-buffer-live-p ediff-meta-buffer)
>  	  (ediff-update-meta-buffer
>  	   ediff-meta-buffer nil ediff-meta-session-number))
> +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)

Would it be ok to avoid the above loop completely by using the buffer
local version of `kill-emacs-hook' instead?


Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 29 Dec 2024 02:26:01 GMT) Full text and rfc822 format available.

Message #40 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Sun, 29 Dec 2024 02:25:16 +0000
[Message part 1 (text/plain, inline)]
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Sent: Saturday, December 28, 2024 05:49 PM
> > +(defun ediff--delete-temp-files-on-kill-emacs ()
> > +  "Delete the temp-files associated with the ediff buffers."
> > +  (ignore-errors
> > +    (let ((inhibit-interaction t))
> > +      (dolist (b (buffer-list))
> > +        (with-current-buffer b
> > +          (when (eq major-mode 'ediff-mode)
> > +            (ediff-delete-temp-files)))))))
> I think this is the same as mapping over `ediff-session-registry' which
>  should hold exactly the list of buffers we want.
>  Second: Can we move the `ignore-errors' inwards so that an error in one
> case doesn't abort the complete loop?

Sure, I had attached the modified one towards your comments.

> >  ;;; Setup functions
> >
> > @@ -488,6 +496,7 @@ ediff-setup
> >        (if (ediff-buffer-live-p ediff-meta-buffer)
> >          (ediff-update-meta-buffer
> >           ediff-meta-buffer nil ediff-meta-session-number))
> > +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
> Would it be ok to avoid the above loop completely by using the buffer
>  local version of `kill-emacs-hook' instead?

I didn't found a local version of `kill-emacs-hook` example in emacs git repo. 
Current version also work for the killing from command line, for example executing a "pkill emacs" during ediff session.
[0001-lisp-vc-ediff-util.el-Loop-the-ediff-session-registr.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 29 Dec 2024 07:05:01 GMT) Full text and rfc822 format available.

Message #43 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: sunlin7 <at> hotmail.com, 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Sun, 29 Dec 2024 09:04:47 +0200
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  "74881 <at> debbugs.gnu.org"
>  <74881 <at> debbugs.gnu.org>
> Date: Sun, 29 Dec 2024 02:49:14 +0100
> 
> Second: Can we move the `ignore-errors' inwards so that an error in one
> case doesn't abort the complete loop?

Binding inhibit-interaction non-nil could cause the body signal an
error, so this should be taken into account if we move ignore-errors
inwards.  This code must NOT signal any errors, ever.  Which AFAIU
means that if we want ignore-errors not to abort the rest of the loop
(why not, btw? these are just temporary files, after all), we should
redesign this loop such that it catches the errors and continues with
the rest of the buffers.

> >  ;;; Setup functions
> >
> > @@ -488,6 +496,7 @@ ediff-setup
> >        (if (ediff-buffer-live-p ediff-meta-buffer)
> >  	  (ediff-update-meta-buffer
> >  	   ediff-meta-buffer nil ediff-meta-session-number))
> > +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
> 
> Would it be ok to avoid the above loop completely by using the buffer
> local version of `kill-emacs-hook' instead?

You assume that the temporary Ediff files are always visited in some
buffer?  Is that assumption true?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sun, 29 Dec 2024 23:34:01 GMT) Full text and rfc822 format available.

Message #46 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Mon, 30 Dec 2024 00:34:22 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

> > > @@ -488,6 +496,7 @@ ediff-setup
> > >        (if (ediff-buffer-live-p ediff-meta-buffer)
> > >  	  (ediff-update-meta-buffer
> > >  	   ediff-meta-buffer nil ediff-meta-session-number))
> > > +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
> >
> > Would it be ok to avoid the above loop completely by using the buffer
> > local version of `kill-emacs-hook' instead?
>
> You assume that the temporary Ediff files are always visited in some
> buffer?  Is that assumption true?

I think you misunderstood: my idea was to put `ediff-delete-temp-files'
to the local hook binding of the ediff control buffers (which we
currently consult anyway).  This would result in the same calls as now -
at least when local `kill-emacs-hook' bindings worked.

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 30 Dec 2024 01:37:02 GMT) Full text and rfc822 format available.

Message #49 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Mon, 30 Dec 2024 02:37:33 +0100
Lin Sun <sunlin7 <at> hotmail.com> writes:

> I didn't found a local version of `kill-emacs-hook` example in emacs
> git repo.

I would expect it to work.  I found one in Gnu Elpa, in "subed.el".

> Current version also work for the killing from command line, for
> example executing a "pkill emacs" during ediff session.

I think this feature would not go.


> diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el
> index 33666535843..d448973b345 100644
> --- a/lisp/vc/ediff-util.el
> +++ b/lisp/vc/ediff-util.el
> @@ -219,12 +219,11 @@ ediff--delete-temp-files-on-kill-emacs
>    ;; where this hook could prevent kill-emacs from shutting down Emacs,
>    ;; because user interaction is not possible (e.g., in a daemon), or
>    ;; if deleting these files signals an error.
> -  (ignore-errors
> -    (let ((inhibit-interaction t))
> -      (dolist (b (buffer-list))
> +  (let ((inhibit-interaction t))
> +    (dolist (b ediff-session-registry)
> +      (ignore-errors
>          (with-current-buffer b
> -          (when (eq major-mode 'ediff-mode)
> -            (ediff-delete-temp-files)))))))
> +          (ediff-delete-temp-files))))))
>  
>  ;;; Setup functions

Yes, this is what I had in mind when the local hook can't be used.


Thank you,

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 30 Dec 2024 02:09:02 GMT) Full text and rfc822 format available.

Message #52 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Mon, 30 Dec 2024 02:07:51 +0000
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Sent: Sunday, December 29, 2024 05:37 PM
> ...
> I would expect it to work.  I found one in Gnu Elpa, in "subed.el".
> ...
> Yes, this is what I had in mind when the local hook can't be used.

Hi Michael, thank you for comments.

And I had tried adding the function to kill-emacs-hook with "local" flag but it does NOT work with kill/pkill emacs during a ediff-session,  I don't know the "local" kill-emacs-hook behavior is a feature or bug.

So the previous patch maybe the suitable one for current situation, it avoid looping all buffers and almost won't affect any user experience.  Thanks



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 30 Dec 2024 13:03:02 GMT) Full text and rfc822 format available.

Message #55 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: sunlin7 <at> hotmail.com, 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Mon, 30 Dec 2024 15:02:31 +0200
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: sunlin7 <at> hotmail.com,  74881 <at> debbugs.gnu.org
> Date: Mon, 30 Dec 2024 00:34:22 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > > > @@ -488,6 +496,7 @@ ediff-setup
> > > >        (if (ediff-buffer-live-p ediff-meta-buffer)
> > > >  	  (ediff-update-meta-buffer
> > > >  	   ediff-meta-buffer nil ediff-meta-session-number))
> > > > +      (add-hook 'kill-emacs-hook #'ediff--delete-temp-files-on-kill-emacs)
> > >
> > > Would it be ok to avoid the above loop completely by using the buffer
> > > local version of `kill-emacs-hook' instead?
> >
> > You assume that the temporary Ediff files are always visited in some
> > buffer?  Is that assumption true?
> 
> I think you misunderstood: my idea was to put `ediff-delete-temp-files'
> to the local hook binding of the ediff control buffers (which we
> currently consult anyway).  This would result in the same calls as now -
> at least when local `kill-emacs-hook' bindings worked.

I don't understand how a buffer-local kill-emacs-hook could work.
Emacs calls the value of this hook only once, when it is going to
exit.  So if the value is buffer-local, whether or not the hook is
called will depend on which buffer is the current buffer when Emacs is
killed.  Or did I miss something?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 30 Dec 2024 22:34:01 GMT) Full text and rfc822 format available.

Message #58 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> hotmail.com, 74881 <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Mon, 30 Dec 2024 23:34:23 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

> > [...] at least when local `kill-emacs-hook' bindings worked.
>
> I don't understand how a buffer-local kill-emacs-hook could work.
> Emacs calls the value of this hook only once, when it is going to
> exit.

I see - then my idea won't work.

Thx,

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 30 Dec 2024 22:37:01 GMT) Full text and rfc822 format available.

Message #61 received at 74881 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lin Sun <sunlin7 <at> hotmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "74881 <at> debbugs.gnu.org" <74881 <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Mon, 30 Dec 2024 23:37:10 +0100
Lin Sun <sunlin7 <at> hotmail.com> writes:

> I don't know the "local" kill-emacs-hook behavior is a feature or bug.

I also don't know, but it seems the behavior of the hook is not special
in any way.

> So the previous patch maybe the suitable one for current situation, it
> avoid looping all buffers and almost won't affect any user experience.

Ok, so let's stick to it.


Thanks,

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Sat, 04 Jan 2025 12:40:01 GMT) Full text and rfc822 format available.

Message #64 received at 74881-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: sunlin7 <at> hotmail.com, 74881-done <at> debbugs.gnu.org
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove
 temp files on kill-emacs
Date: Sat, 04 Jan 2025 14:39:35 +0200
> From: Michael Heerdegen <michael_heerdegen <at> web.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  "74881 <at> debbugs.gnu.org"
>  <74881 <at> debbugs.gnu.org>
> Date: Mon, 30 Dec 2024 23:37:10 +0100
> 
> Lin Sun <sunlin7 <at> hotmail.com> writes:
> 
> > I don't know the "local" kill-emacs-hook behavior is a feature or bug.
> 
> I also don't know, but it seems the behavior of the hook is not special
> in any way.
> 
> > So the previous patch maybe the suitable one for current situation, it
> > avoid looping all buffers and almost won't affect any user experience.
> 
> Ok, so let's stick to it.

Installed on master, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#74881; Package emacs. (Mon, 06 Jan 2025 07:16:02 GMT) Full text and rfc822 format available.

Message #67 received at 74881-done <at> debbugs.gnu.org (full text, mbox):

From: Lin Sun <sunlin7 <at> hotmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: "74881-done <at> debbugs.gnu.org" <74881-done <at> debbugs.gnu.org>
Subject: Re: bug#74881: 31.0.50; [PATCH] * lisp/vc/ediff-util.el: Remove temp
 files on kill-emacs
Date: Mon, 6 Jan 2025 07:15:43 +0000
> From: Eli Zaretskii <eliz <at> gnu.org>
> Sent: Saturday, January 4, 2025 12:39 PM
> Installed on master, thanks.

Thank you, appreciate it. 



bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 03 Feb 2025 12:24:15 GMT) Full text and rfc822 format available.

This bug report was last modified 134 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.