[Bug] (highlight-delay-load) Window-state-change leaking

[akashpal-21]

When org-remark-mode is active - there is a delay load mechanism for the highlights - this delay load is achieved via utilising window-state-change-functions -

If buffer in question is opened directly instead of using find-file -- during new client frame creation - this leaks to ALL open buffers if the mode is activated by using local evaluation. such as

# Local Variables:
# eval: (org-remark-mode)
# End:

This is problematic since then it will try to load highlights for all buffers whether org-remark-mode is active or not in them. Normally this opens the notes.org file for each buffer concerned due to the logic inside org-remark-highlights-load -- since (org-remark-notes-get-file-name) is called inside the code -- this creates the notes-buffer for the buffers automatically.

One solution to make the delay load more robust is to use a local post-command-hook -- during tests - this doesn't leak while still giving the same benefits

(defun patch/org-remark-highlights-delay-load ()
  "Delay load until WINDOW for current buffer is created."
  (when (get-buffer-window)
    (remove-hook 'post-command-hook
         #'org-remark-highlights-delay-load 'local)
    (org-remark-highlights-load)))

(defun patch/org-remark-highlights-load (&optional update)
  "Visit notes file & load the saved highlights onto current buffer.
If there is no highlights or annotations for current buffer,
output a message in the echo.

Non-nil value for UPDATE is passed for the notes-source sync
process."
  (if (not (get-buffer-window))
      (add-hook 'post-command-hook
                #'org-remark-highlights-delay-load 95 'local)
    ;; Some major modes such as nov.el reuse the current buffer, deleting
    ;; the buffer content and insert a different file's content. In this
    ;; case, obsolete highlight overlays linger when you switch from one
    ;; file to another. Thus, in order to update the highlight overlays we
    ;; need to begin loading by clearing them first. This way, we avoid
    ;; duplicate of the same highlight.
    (org-remark-highlights-clear)
    ;; Loop highlights and add them to the current buffer
    (let (overlays) ;; highlight overlays
      (when-let* ((notes-filename (org-remark-notes-get-file-name))
                  (default-dir default-directory)
                  (notes-buf (or (find-buffer-visiting notes-filename)
                                 (find-file-noselect notes-filename)))
                  (source-buf (current-buffer)))
        (with-demoted-errors
            "Org-remark: error during loading highlights: %S"
          ;; Load highlights with demoted errors -- this makes the loading
          ;; robust against errors in loading.
          (dolist (highlight (org-remark-highlights-get notes-buf))
            (let ((ov (org-remark-highlight-load highlight)))
              (when ov (push ov overlays))))
          (unless update (org-remark-notes-setup notes-buf source-buf))
          (if overlays
              (progn (run-hook-with-args 'org-remark-highlights-after-load-functions
                                         overlays notes-buf)
                     ;; Return t
                     t)
            ;; if there is no overlays loaded, return nil
            nil))))))

(advice-add 'org-remark-highlights-delay-load :override #'patch/org-remark-highlights-delay-load)
(advice-add 'org-remark-highlights-load :override #'patch/org-remark-highlights-load)

I show an example how to use this within the existing code.

Best,

[nobiot]

@akashpal-21 Thank you. Merged the patch to the main, which will be sync'ed with ELPA-devel in the next cycle. Let's keep it there for now to see if there is any regression that we have missed so far.

[akashpal-21]

Closing this issue - please reopen it if any regressions surface 🙂 . Thank you.