Hi Paul, Thanks for this! I'll address your comments in the next patch, but some questions/clarifications below.
On 05/01/2026, Paul D. Nelson wrote: > Regarding the issue of custom setters for the obsoleted variable > preview-leave-open-previews-visible, here's a diff (on top of your > latest patch) indicating how this might be done: Since you seem to be wanting the setting of `preview-keep-stale-images` to change the variables only when set to non-nil, then perhaps a custom setter is sufficient after all? Or are you wanting to handle setq as well? > I remember some discussion about the use of cl-* functions (not macros), > see https://lists.gnu.org/archive/html/bug-auctex/2024-04/msg00117.html > and follow-ups. Others will know better than I do whether we would need > to add cl-lib to Packages-Requires, etc. OK, I'll just use delq > Is it clear why this should be a macro? Wouldn't a defun be easier to > debug? I also think a function is better, but since it is a thin wrapper around the macro `preview-make-clickable` I thought a macro is more appropriate. Happy to change it. > This returns a single string, so the pluralization seems unclear. Also, > this function doesn't seem to be used anymore. Sorry, that was a typo, it should `preview-disabled-string`. This is an old obsolete function. > This last sentence (starting "ARG ...") seems a bit garbled to me. Indeed, it should be ARG can also be `maybe-open' to display the underlying text only when the cursor of the overlay's buffer is inside the overlay (in this case, OV is added to `preview-temporary-opened'). >> +(defun preview-overlay-updated (ov) >> + "Mark preview OV as updated." >> + (with-current-buffer (window-buffer) > > What if a preview is updated via some async path while you're working in > another buffer/window/frame? Are there some assumptions about how this > function is called? Would it be more robust to use (overlay-buffer ov) > instead of (window-buffer)? It would be difficult for me to test all possible async paths. But the purpose of this statement is to select the truly selected buffer in the window, rather than a temporarily selected one since `preview-overlay-updated` itself is/might be called inside a different `with-current-buffer`. I can't use (overlay-buffer ov) since the point is for `preview--update-buframe` to display the buframe if the currently selected buffer in the window is equal to the overlay buffer. > I believe the convention is to prefer the imperative: "Return the > started process." > I copied this verbatim from `preview-generate-preview` (the last form in the function). Should I change both? > Also, just checking: are there any failure paths along which it doesn't > start the process, or return it? I don't see any, short of an error or if `preview-generate-preview` itself does not return the process, in which case the documentation of that function is itself wrong. > I guess it'd be cleaner if this were "buframe-find", but OK. Indeed. I now rename this function in buframe and will push a version bump soon. I also fixed the issue of the frame placement with the multi-monitor, if you can pull the change and test it on your setup. Thanks, -- Al _______________________________________________ bug-auctex mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-auctex
