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