Hi Al, I've started taking a look and have several comments.
Thanks, best, Paul > +(defvar-local preview-silent-errors nil > + "When non-nil, do not signal preview errors nor display output buffer. > +This variable should be set in the process buffer.") (Very good.) > +;;; preview-point customizations and variables. > +(defcustom preview-point nil > + "Specifies where to show the preview. > +If non-nil, show the preview at point. Can be `before-string', > +`after-string' to show at before or after the TeX code or `buframe' to > +show in a separate frame (the `buframe' package must be installed). 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. > + > +This variable must be set before opening a TeX file. Charging its value > +while a file is open will lead to previews not being displayed > +correctly. See `preview-point-toggle-mode'." > + :type '(choice > + (const :tag "Before string" before-string) > + (const :tag "After string (default)" after-string) > + (const :tag "On frame" buframe) > + (list :tag "On frame with explicit parameters" > + (function :tag "Position function") > + (alist :tag "Frame parameters") > + (alist :tag "Buffer parameters")))) I have several unrelated comments: Documentation: preview-point seems to work fine in non-file buffers, so perhaps replace "TeX file" with "TeX buffer". Typo: "Charging its value" -> "Changing its value" The default is nil, but ':type' doesn't accept nil. Consider adding (const :tag "Disabled" nil). It would be much more ergonomic if we could enable preview-point after visiting a buffer. Is this a hard limitation? > +(defcustom preview-point-auto-p nil > + "Set this to enable previewing automatically. > +When non-nil, it is assumed to be a function that is is called with no > +arguments at (point) when there is not a preview already. If the > +function return a non-nil value, a `preview-at-point' will be initiated." > + :type 'symbol) The documentation does not indicate any intended value. Also, from what I understand, this setting controls when new previews are generated, but does not affect whether existing previews are automatically regenerated; the docs don't seem to emphasize this distinction. > +(defun preview-point-toggle-mode (mode) > + "Set `preview-point' to MODE. > +If called interactively `preview-point' is toggled and the user is asked > +for the MODE when it is switch on. > + > +This function has an effect only if `preview-point' is non-nil, which > +should be set before opening a TeX file." > + (interactive > + (list (if preview-point > + (if (not (eq preview-point 'hidden)) > + 'hidden > + (intern > + (cadr (read-multiple-choice > + "Mode :" > + '((?b "before-string") > + (?a "after-string") > + (?f "buframe")))))) > + (error "Preview-point is not enabled. \ > +Set preview-point before opening the TeX file")))) > + > + (unless preview-point > + (error "Preview-point needs to be enabled first (before opening the tex > file).")) > + ;; Hide current overlay if it is visible. > + (when preview-point--current-overlay > + (preview-toggle preview-point--current-overlay nil)) > + (setq preview-point mode) > + ;; Show the preview if it at point. > + (preview-point-move-point)) The command ends in "-mode" but does not come from a major/minor mode. Perhaps instead something like "preview-point-toggle-display"? It might be cleaner to move the body of this command to a custom setter for preview-point and then either delete the command altogether, or rewrite it as a light wrapper around customize-set-variable. That way, users can tweak preview-point via either the new C-c C-p C-t bind or customize-set-variable. > +(defun preview-point-toggle-auto-update (enable &optional silent) > + "Enable auto refresh of previews. > +When ENABLE is nil, disable auto-update instead. If called > +interactively, auto-updating is toggled." > + (interactive (list 'toggle)) > + (when (eq enable 'toggle) > + (setq enable (not (memq > + #'preview-point-buf-change > + after-change-functions)))) > + (if enable > + (progn > + (add-hook 'after-change-functions #'preview-point-buf-change nil t) > + (add-hook 'post-command-hook #'preview-point-auto nil t) > + (preview-point-buf-change) > + (unless silent > + (message "Auto-updating previews enabled"))) > + (remove-hook 'after-change-functions #'preview-point-buf-change t) > + (remove-hook 'post-command-hook #'preview-point-auto t) > + (unless silent > + (message "Auto-updating previews disabled")))) This reads like a minor mode. I would suggest introducing a small, display-agnostic "preview-automatic-mode", bound to C-c C-p C-a, that, when enabled, automatically regenerates existing previews, for both preview-point and "traditional" preview. This aligns with an earlier suggestion [1] to upstream automatic previewing. (It's simpler than preview-auto, which also *finds* new regions to preview, but that feature could be added later via a strategy variable or hook.) I'd be happy to draft that refactor on top of your patch if the idea sounds reasonable. [1] https://lists.gnu.org/archive/html/auctex-devel/2024-06/msg00031.html > + > +;;; preview-point -- Auto-preview If we refactor as suggested above, then most of the functions below should be given agnostic names ("preview-automatic-*"?), reflecting that they are not specific to the preview-point feature. > +(defcustom preview-point-auto-delay 0.1 > + "Delay in seconds for automatic preview timer." > + :type 'number) > +(defvar preview-point-force-update--debounce-timer nil) > + > +(defun preview-point@around@write-region (orig-fun &rest args) > + "Advice around `write-region' to suppress messages. > +ORIG-FUN is the original function. ARGS are its arguments." > + (let ((noninteractive t) > + (inhibit-message t) > + message-log-max) > + (apply orig-fun args))) > + > +(defun preview-point-force-update (pt buffer &optional debounce) > + "Update preview at PT in BUFFER. > + > +When DEBOUNCE is non-nil, the call is debounced using an idle > +timer. This also happens automatically when there is an ongoing > +compilation process." > + (interactive (list (point) (current-buffer))) > + > + (when preview-point-force-update--debounce-timer > + (cancel-timer preview-point-force-update--debounce-timer) > + (setq preview-point-force-update--debounce-timer nil)) > + > + (when (buffer-live-p buffer) > + (unless debounce > + (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)))))) > + (progn > + ;; Force de-bouncing > + (when (and preview-current-region > + (not preview-abort-flag) > + ;; (< beg (cdr preview-current-region)) > + ) > + (progn > + (ignore-errors (TeX-kill-job)) > + (setq preview-abort-flag t))) > + (with-local-quit (accept-process-output cur-process)) > + (setq debounce t)) > + ;; The code below is adopted from preview-auto The code above this comment also appears to be derived from preview-auto (see [2]), so if such a comment is to appear at all, then it should be higher up (e.g., after ;;; preview-point -- Auto-preview). But my impression is that attribution is more customarily indicated in commit messages, so I would propose removing such comments from the code and instead including in the commit message something like the following: --8<---------------cut here---------------start------------->8--- Automatic previewing code derived from preview-auto.el (Copyright 2024 FSF, by Paul D. Nelson), adapted and integrated by Al-Haji Ali. --8<---------------cut here---------------end--------------->8--- [2] https://lists.gnu.org/archive/html/auctex-devel/2025-08/msg00047.html > + (let ((TeX-suppress-compilation-message t) > + (save-silently t)) > + (advice-add 'write-region :around > + #'preview-point@around@write-region) > + (unwind-protect > + ;; If we are working in a file buffer that is not a tex file, > + ;; then we want preview-region to operate in "non-file" mode, > + ;; where it passes "<none>" to TeX-region-create. > + (save-excursion > + (goto-char pt) > + ;; TODO: Check if we can rely on the `preview-region' > + ;; returning the process. It calls > + ;; `preview-generate-preview' which has this documented > + ;; behaviour, but not `preview-region'. > + (let ((process (preview-region (preview-next-border t) > + (preview-next-border nil)))) > + (with-current-buffer (process-buffer process) > + (setq-local preview-silent-errors t)))) > + (advice-remove 'write-region > + #'preview-point@around@write-region)))))) > + > + (when debounce > + (setq preview-point-force-update--debounce-timer > + (run-with-idle-timer > + preview-point-auto-delay nil > + #'preview-point-force-update > + pt buffer))))) > + > +(defun preview-point-has-preview-p (&optional pt) > + "Return non-nil if PT has a preview overlay." > + (cl-find-if > + (lambda (ov) (overlay-get ov 'preview-state)) > + (overlays-at (or pt (point))))) > + > +(defun preview-point-buf-change (&rest _) > + "Run preview at point if there is a preview overlay." > + (when (and preview-point > + (not (eq preview-point 'hidden)) > + (or > + (preview-point-has-preview-p) > + (and preview-point-auto-p > + (funcall preview-point-auto-p)))) > + (preview-point-force-update (point) (current-buffer) t))) > + > ;;;###autoload > (defun preview-report-bug () "Report a bug in the preview-latex package." > (interactive) As a final comment, thanks for working on this and for your other recent improvements to preview.el - much appreciated! _______________________________________________ bug-auctex mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-auctex
