Hi Al,

Al Haji-Ali <abdo.haji....@gmail.com> writes:

> I just made public an auctex preview package that I have been working
> on for some time. It essentially shows the preview locally and
> temporarily in a frame or as an 'after-string, similar to what
> Overleaf has been doing recently.
>
> https://github.com/haji-ali/preview-point
>
> Any feedback/bug reports are appreciated.

many thanks for creating and sharing this.  I'm not a heavy
preview-latex user, but I do like your approach since I tend to look at
code at all the time, so your `buframe' version matches my workflow
more.

I briefly tried and looked at the code; please find attached my proposed
changes.  Mostly docstring and changes from when-let/if-let to the
starred version.

Another question: How do want to proceed once the code is more mature?
ELPA package or add it to AUCTeX?

> One of the "helper" packages that I wrote is `preview-dvi` which fixes
> what I think are several issues with preview related to robustness and
> ensuring files/folders are not orphaned in the preview directory.
>
> In addition to plugging my package here, I wanted to check if these
> are indeed real issues, and if there is interest in implementing those
> fixes in preview.el. I'll try to summarize the issues here. The fixes
> are all marked by "NOTE" in my preview-dvi.el

I hope Keita (in Cc) chimes in here; he is more familiar with preview
code.

Best, Arash
diff --git a/buframe.el b/buframe.el
index e7c2e49..f703931 100644
--- a/buframe.el
+++ b/buframe.el
@@ -158,7 +158,7 @@ If NOERROR is nil and no frame is found, signal an error."
             frame-or-name)
      (cl-find-if
       (lambda (frame)
-        (when-let ((buffer-info (frame-parameter frame 'buframe)))
+        (when-let* ((buffer-info (frame-parameter frame 'buframe)))
           (and
            (or (null frame-or-name)
                (equal (frame-parameter frame 'name) frame-or-name))
@@ -181,10 +181,9 @@ If NOERROR is nil and no frame is found, signal an error."
   "Create or reuse a child FRAME displaying BUFFER, positioned using FN-POS.
 
 By default, the frame is configured to be minimal, dedicated,
-non-focusable, and properly sized to its buffer. Positioning is
-delegated to FN-POS. If an existing child frame matching
-FRAME-OR-NAME and BUFFER exists, it is reused; otherwise, a new
-one is created.
+non-focusable, and properly sized to its buffer.  Positioning is
+delegated to FN-POS.  If an existing child frame matching FRAME-OR-NAME
+and BUFFER exists, it is reused; otherwise, a new one is created.
 
 FRAME-OR-NAME is either the frame to reuse or its name.
 FN-POS is a function called with the frame and overlay/position,
@@ -278,7 +277,7 @@ Also ensure frame is made visible."
                (frame-live-p frame)
                (not (buframe-disabled-p frame)))
       (with-current-buffer (plist-get info :parent-buffer)
-        (if-let (pos (funcall fn-pos frame))
+        (if-let* (pos (funcall fn-pos frame))
             (pcase-let ((`(,px . ,py) (frame-position frame))
                         (`(,x . ,y) pos))
               (unless (and (= x px) (= y py))
@@ -297,7 +296,7 @@ Also ensure frame is made visible."
 (defun buframe-disable (frame-or-name &optional enable)
   "Disable and hide FRAME-OR-NAME.
 If ENABLE is non-nil, re-enable and show it."
-  (when-let (frm (buframe--find frame-or-name))
+  (when-let* (frm (buframe--find frame-or-name))
     (when (frame-live-p frm)
       (set-frame-parameter
        frm 'buframe
@@ -311,7 +310,7 @@ If ENABLE is non-nil, re-enable and show it."
 
 (defun buframe-hide (frame-or-name)
   "Make FRAME-OR-NAME invisible."
-  (when-let (frm (buframe--find frame-or-name))
+  (when-let* (frm (buframe--find frame-or-name))
     (when (and (frame-live-p frm)
                (frame-visible-p frm))
       (make-frame-invisible frm)))
@@ -383,7 +382,7 @@ BUFFER can be:
   \\='not-parent  – run only if parent buffer is not current
   a buffer     – run only if BUFFER is current."
   (if frame-or-name
-      (when-let (frame (buframe--find frame-or-name))
+      (when-let* (frame (buframe--find frame-or-name))
         (let ((is-parent (equal (window-buffer)
                                 (plist-get (frame-parameter frame 'buframe)
                                            :parent-buffer))))
@@ -394,7 +393,7 @@ BUFFER can be:
             (funcall fn frame))))
     (cl-mapc
      (lambda (frame)
-       (when-let ((buffer-info (frame-parameter frame 'buframe)))
+       (when-let* ((buffer-info (frame-parameter frame 'buframe)))
          (buframe--auto* frame fn buffer)))
      (frame-list))))
 
diff --git a/preview-dvi.el b/preview-dvi.el
index 0cc81a5..9bb90e8 100644
--- a/preview-dvi.el
+++ b/preview-dvi.el
@@ -54,7 +54,8 @@
 
 (defvar preview-dvi-after-place-hook nil
   "Hook run after preview overlays are placed.
-Each function is called with one argument: the list of overlays processed.")
+Each function is called with one argument: the list of overlays
+processed.")
 
 (defvar preview-dvi-config-default
   '((image-type png)
@@ -90,7 +91,7 @@ Values are overridden by entries in `preview-image-creators'.")
             type))))
 
 (defun preview-dvi-start ()
-  "Start a Dvi conversion process process.
+  "Start a dvi conversion process process.
 See the original `preview-start-dvipng'."
   (let* ((name (preview-dvi-config 'process-name))
          (dir-command (funcall (preview-dvi-config 'command)))
@@ -123,7 +124,7 @@ See the original `preview-start-dvipng'."
   "Place all images dvipng has created, if any.
 Deletes the dvi file when finished.
 
-See the original `preview-dvipng-place-all'"
+See the original `preview-dvipng-place-all'."
   (let ((type (preview-dvi-config 'image-type))
         filename queued oldfiles snippet processed)
     (dolist (ov (prog1 preview-gs-queue (setq preview-gs-queue nil)))
@@ -203,7 +204,7 @@ See the original `preview-dvipng-abort'"
 The usual PROCESS and COMMAND arguments for
 `TeX-sentinel-function' apply.  Places all snippets if PLACEALL
 is set.
-See the original `preview-dvipng-sentinel'"
+See the original `preview-dvipng-sentinel'."
   (condition-case err
       (let ((status (process-status process)))
         (cond ((or (eq status 'signal)
@@ -223,7 +224,7 @@ See the original `preview-dvipng-sentinel'"
 
 (defun preview-dvi-close (process closedata)
   "Clean up after PROCESS and set up queue accumulated in CLOSEDATA.
-See the original `preview-dvipng-abort'"
+See the original `preview-dvipng-abort'."
   (setq preview-gs-queue (nconc preview-gs-queue closedata))
   (if process
       (if preview-gs-queue
@@ -282,27 +283,27 @@ deletions. Return list of overlays placed."
       ;; have the filename of our preview image in their 'filename property to
       ;; avoid eager file deletion
       (when preview-leave-open-previews-visible
-        (when-let (filename (cadr (overlay-get ov 'preview-image)))
-        (let ((start (or (overlay-start ov) (point-min)))
-              (end (or (overlay-end ov) (point-max)))
-              (exception ov)
+        (when-let* (filename (cadr (overlay-get ov 'preview-image)))
+          (let ((start (or (overlay-start ov) (point-min)))
+                (end (or (overlay-end ov) (point-max)))
+                (exception ov)
                 (timestamp (overlay-get ov 'timestamp)))
-          (dolist (oov (overlays-in start end)) ;; Old overlays
-            (when (and (not (eq oov exception))
-                       (overlay-get oov 'preview-state)
-                       (not (and timestamp
-                                 (equal timestamp (overlay-get oov
-                                                               'timestamp)))))
-              ;; The overlay is gonna be deleted with its files.
-              ;; Make sure its `filenames' does not contain our image
-              (let ((files-oov (overlay-get oov 'filenames))
-                    (files-ov  (overlay-get ov  'filenames)))
-                (when-let ((entry (assoc filename files-oov)))
-                  (overlay-put oov '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)
+            (dolist (oov (overlays-in start end)) ;; Old overlays
+              (when (and (not (eq oov exception))
+                         (overlay-get oov 'preview-state)
+                         (not (and timestamp
+                                   (equal timestamp (overlay-get oov
+                                                                 'timestamp)))))
+                ;; The overlay is gonna be deleted with its files.
+                ;; Make sure its `filenames' does not contain our image
+                (let ((files-oov (overlay-get oov 'filenames))
+                      (files-ov  (overlay-get ov  'filenames)))
+                  (when-let* ((entry (assoc filename files-oov)))
+                    (overlay-put oov '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)))))))))))
     ovl))
 
@@ -336,8 +337,8 @@ but then you'll need to adapt `preview-dvi-config'."
 
 (defun preview-dvisvgm-command ()
   "Return a shell command for running dvisvgm.
-Result is a cons cell (COMMAND . TEMPDIR). Includes scaling based
-on preview magnification and text-scale settings."
+Result is a cons cell (COMMAND . TEMPDIR).  Includes scaling based on
+preview magnification and text-scale settings."
   (let* (;; (file preview-gs-file)
          (scale (* (/ (preview-hook-enquiry preview-scale)
                       (preview-get-magnification))
@@ -353,7 +354,7 @@ on preview magnification and text-scale settings."
 ;;; Updates to variables.
 (defun preview-dvi-variable-standard-value (symbol)
   "Return the standard value of variable SYMBOL.
-  This looks up SYMBOL's `standard-value' property and evaluates it."
+This looks up SYMBOL's `standard-value' property and evaluates it."
   (let ((container (get symbol 'standard-value)))
     (cl-assert (consp container) "%s does not have a standard value")
     (eval (car container))))
@@ -361,7 +362,7 @@ on preview magnification and text-scale settings."
 (defun preview-dvi-set-variable-standard-value (symbol value)
   "Set standard value of SYMBOL to VALUE.
 If SYMBOL currently holds its standard value, also set it to VALUE.
-Update SYMBOL's `standard-value' property accordingly"
+Update SYMBOL's `standard-value' property accordingly."
   (let ((standard (preview-dvi-variable-standard-value symbol)))
     (when (equal (symbol-value symbol) standard)
       (set symbol value))
diff --git a/preview-point.el b/preview-point.el
index 7d7d6d6..ee06949 100644
--- a/preview-point.el
+++ b/preview-point.el
@@ -38,12 +38,11 @@
 
 (defcustom preview-point-show-in 'after-string
   "Specifies where to show the preview.
-Can be `before-string', `after-string' or `buframe'. Can also be
-\\='(buframe FN-POS FRAME-PARAMETERS BUF-PARAMETERS) where FN-POS
-is a position function (default is
-`buframe-position-right-of-overlay') and FRAME-PARAMETERS is an
-alist of additional frame parameters, default is nil and
-BUF-PARAMETERS is an alist of buffer local variables and their
+Can be `before-string', `after-string' or `buframe'.  Can also be
+\\='(buframe FN-POS FRAME-PARAMETERS BUF-PARAMETERS) where FN-POS is a
+position function (default is `buframe-position-right-of-overlay') and
+FRAME-PARAMETERS is an alist of additional frame parameters, default is
+nil and BUF-PARAMETERS is an alist of buffer local variables and their
 values."
   :type '(choice
           (const :tag "Before string" before-string)
@@ -185,12 +184,15 @@ Adapted from `preview-check-changes' to call
 
 (defun preview-point-toggle (ov &optional arg event show-construct)
   "Toggle visibility of preview overlay OV.
-ARG can be one of the following: t activate the preview, nil
-deactivate the preview, `toggle' toggles. If the preview is
-disabled, the disabled symbol is shown when activated. If EVENT
-is given, it indicates the window where the event occurred,
+ARG can be one of the following:
+ t        activate the preview,
+ nil      deactivate the preview,
+ `toggle' toggles.
+
+If the preview is disabled, the disabled symbol is shown when activated.
+If EVENT is given, it indicates the window where the event occurred,
 either by being a mouse event or by directly being the window in
-question. This may be used for cursor restoration purposes.
+question.  This may be used for cursor restoration purposes.
 SHOW-CONSTRUCT forces showing under-construction previews."
   (let* ((old-urgent (preview-remove-urgentization ov))
          (pt (and
@@ -284,7 +286,7 @@ Toggle previews as point enters or leaves overlays."
     (let* ((pt (point))
            (lst (overlays-at pt)))
       ;; Hide any open overlays
-      (when-let (ov preview-point--frame-overlay)
+      (when-let* (ov preview-point--frame-overlay)
         (and (overlay-buffer ov)
              (overlay-get ov 'preview-state)
              (not (eq (overlay-get ov 'preview-state) 'inactive))
@@ -378,11 +380,11 @@ ORIG-FUN is the original function.  ARGS are its arguments."
   (interactive (list (current-buffer)))
   (when (buffer-live-p buffer)
     (with-current-buffer buffer
-      (if-let ((cur-process
-                (or (get-buffer-process (TeX-process-buffer-name
-                                         (TeX-region-file)))
-                    (get-buffer-process (TeX-process-buffer-name
-                                         (TeX-master-file))))))
+      (if-let* ((cur-process
+                 (or (get-buffer-process (TeX-process-buffer-name
+                                          (TeX-region-file)))
+                     (get-buffer-process (TeX-process-buffer-name
+                                          (TeX-master-file))))))
           (let ((preview-point-auto-delay
                  (if (> preview-point-auto-delay 0)
                      preview-point-auto-delay

Reply via email to