Hi Al,

Al Haji-Ali <[email protected]> writes:

> This is a follow-up bug-report/patch from:
> https://lists.gnu.org/archive/html/auctex-devel/2025-08/msg00026.html
>
> This patch makes several changes in cases where certain scenarios lead
> to temporary preview files being left behind. These fall into two
> categories:
> 1. Cases in which files are left behind due to some failure. 
> 2. Cases in which the properties 'queued or 'filenames are overwritten
> without deleting the files which these properties point to.

Thanks, I have some minor comments, see below.

> From 565f387a9b181a588857055bef9d54c57f776813 Mon Sep 17 00:00:00 2001
> From: Al Haji-Ali <[email protected]>
> Date: Wed, 3 Sep 2025 15:41:46 +0100
> Subject: [PATCH] preview: Prevent orphaning of preview files
>
> * preview.el (preview-gs-sentinel): Delete preview.ps file when process
> is not restarted.
> (preview-dvips-abort): Delete temporary directory if not used.
> (preview-gs-place): Delete old files before overwriting 'filenames. Save
> filename in new overlay when `preview-leave-open-previews-visible' is
> non-nil.
> (preview--delete-overlay-files): New function.
> (preview-disable preview-delete): Use function above.
> (preview-dvipng-place-all): Do not add `preview-ps-file' twice in
> filenames and always delete oldfiles.
> ---
>  preview.el | 148 +++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 99 insertions(+), 49 deletions(-)
>
> diff --git a/preview.el b/preview.el
> index 22316335..91303db4 100644
> --- a/preview.el
> +++ b/preview.el
> @@ -637,7 +637,8 @@ Emacs color well."
>  Gets the default PROCESS and STRING arguments
>  and tries to restart Ghostscript if necessary."
>    (condition-case err
> -      (let ((status (process-status process)))
> +      (let ((status (process-status process))
> +            keep-preview-ps)
>          (when (memq status '(exit signal))
>            (setq compilation-in-progress (delq process 
> compilation-in-progress)))
>          (when (buffer-name (process-buffer process))
> @@ -662,18 +663,12 @@ and tries to restart Ghostscript if necessary."
>                                   (point-max)))
>                      (insert-before-markers err)))
>                  (delete-process process)
> -                (if (or (null ov)
> +                (unless (or (null ov)
>                          (eq status 'signal))

The indentation looks odd, shouldn't the above look like this:

(unless (or (null ov)
            (eq status 'signal))

?

> [...]
>    (overlay-put ov 'queued
>                 (vector box nil snippet))
> -  (overlay-put ov 'preview-image
> -               (let ((default (list (preview-icon-copy 
> preview-nonready-icon))))
> -                 (if preview-leave-open-previews-visible
> -                     (if-let* ((img
> -                                (car
> -                                 (delq
> -                                  nil
> -                                  (mapcar
> -                                   (lambda (ovr)
> -                                     (and
> -                                      (eq (overlay-start ovr) (overlay-start 
> ov))
> -                                      (overlay-get ovr 'preview-image)))
> -                                   (overlays-at (overlay-start ov)))))))
> -                         img
> -                       default)
> -                   default)))
> +
> +  (if-let* ((old-ov
> +             (and preview-leave-open-previews-visible
> +                  (car
> +                   (delq
> +                    nil
> +                    (mapcar
> +                     (lambda (ovr)
> +                       (and
> +                        (eq (overlay-start ovr) (overlay-start ov))
> +                        (overlay-get ovr 'preview-image)
> +                        ovr))
> +                     (overlays-at (overlay-start ov))))))))
> +      (progn

Is this `progn' necessary?

> +        (let* ((img (overlay-get old-ov 'preview-image))
> +               (filename (cadr img))
> +               (files-oov (overlay-get old-ov 'filenames))
> +               (files-ov  (overlay-get ov  'filenames)))
> +          (when img
> +            (overlay-put ov 'preview-image img)
> +            ;; Transfer filename ownership to new overlay. The old one
> +            ;; will be cleared out and its files deleted.
> +            (when-let* ((entry (assoc filename files-oov)))
> +              (overlay-put old-ov 'filenames
> +                           (assq-delete-all filename files-oov))
> +              ;; Add the filename to the current overlay instead
> +              ;; if it's not already there
> +              (unless (assoc filename files-ov)
> +                (overlay-put ov 'filenames
> +                             (cons entry files-ov)))))))
> +    (overlay-put ov 'preview-image
> +                 (list (preview-icon-copy preview-nonready-icon))))
> +
>    (preview-add-urgentization #'preview-gs-urgentize ov run-buffer)
>    (list ov))

Can you please check the indentation of the code you've added?
Otherwise, I think we're good to go.

Best, Arash



_______________________________________________
bug-auctex mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/bug-auctex

Reply via email to