nobiot / org-remark

Highlight & annotate text, EWW, Info, and EPUB
https://nobiot.github.io/org-remark/
GNU General Public License v3.0
416 stars 20 forks source link

Bug: Error during save if source-buffer is narrowed (args out of range) #83

Open akashpal-21 opened 2 weeks ago

akashpal-21 commented 2 weeks ago

During save - if the source-buffer in question is narrowed - then the org-remark-highlights-adjust-positions function which runs during this time -- probably re initiating all the highlights - fails to check the whencondition that is there to evaluate whether "original text exists AND it is different to the current text" .

Two ways to avoid the problem

Approach one

(defun org-remark-highlights-adjust-positions (overlays _notes-buf)
  "Run dolist and delgate the actual adjustment to another function.

OVERLAYS are highlights.

Check the original text property exits and not the same as the
current highlighted text.

Meant to be set to `org-remark-highlights-after-load-functions' by mode-specific
extensions."
  (dolist (ov overlays)
    (let ((highlight-text (overlay-get ov '*org-remark-original-text)))
      ;; Check that the original text exists AND it is different to the
      ;; current text
      (when
          (and (org-remark-highlights-adjust-positions-p (overlay-get ov 'org-remark-type))
               highlight-text
               (not (org-remark-string=
                     highlight-text
             ;; HHHH---------------------------------------------------
             (when (eq major-mode 'org-mode)
               (org-with-wide-buffer (buffer-substring-no-properties
                          (overlay-start ov) (overlay-end ov))))
             ;; (buffer-substring-no-properties
             ;; (overlay-start ov) (overlay-end ov))
             ;; HHHH---------------------------------------------------
             )))
        (org-remark-highlight-adjust-position-after-load
         ov highlight-text)))))

Since the error occurs during (buffer-substring-no-properties (overlay-start ov) (overlay-end ov)) we may run this within (org-with-wide-buffer

The second approach that I have discovered is changing the after-save-hook

(defun org-remark-save ()
  "Save all the highlights tracked in current buffer to notes buffer.

This function is automatically called when you save the current
buffer via `after-save-hook'.

`org-remark-highlights' is the local variable that tracks every highlight
in the current buffer.  Each highlight is an overlay."
  (interactive)
  (org-remark-highlights-housekeep)
  (org-remark-highlights-sort)
  (let ((notes-buf (find-file-noselect (org-remark-notes-get-file-name)))
        (source-buf (or (buffer-base-buffer) (current-buffer))))
    (dolist (h org-remark-highlights)
      (org-remark-highlight-add h source-buf notes-buf))
    ;;; Avoid saving the notes buffer if it is the same as the source buffer
    (if (eq source-buf notes-buf)
        (set-buffer-modified-p nil)

;; HHHH---------------------------------------------------
      (with-current-buffer source-buf
    (org-with-wide-buffer
;; HHHH---------------------------------------------------
     (with-current-buffer notes-buf
           (save-buffer)))))))

The error in question is as follows

Saving file /home/akash/roam/test.org...
Wrote /home/akash/roam/test.org
Saving file /home/akash/roam/resources/test/notes.org...
Wrote /home/akash/roam/resources/test/notes.org
Org-remark: error during loading highlights: (args-out-of-range #<buffer test.org> 110 119)

Thank you.

akashpal-21 commented 2 weeks ago
(defun org-remark-save ()
    (interactive)
    (org-remark-highlights-housekeep)
    (org-remark-highlights-sort)
    (let ((notes-buf (find-file-noselect (org-remark-notes-get-file-name)))
          (source-buf (or (buffer-base-buffer) (current-buffer))))

      ;; HHHH---------------------------------------------------
      (with-current-buffer source-buf
    (save-restriction
      (widen)
      ;; HHHH---------------------------------------------------

      (dolist (h org-remark-highlights)
        (org-remark-highlight-add h source-buf notes-buf))
      ;; Avoid saving the notes buffer if it is the same as the source buffer
      (if (eq source-buf notes-buf)
              (set-buffer-modified-p nil)
        (with-current-buffer notes-buf
              (save-buffer)))))))

After some more tests - the save-restriction with widen has to be done a little above - otherwise the narrowing will also create problems for org-remark-notes-set-properties especially the :org-remark-link: will point to the wrong line number

I also tried messing around with the after-save-hook for the notes buffer - aka org-remark-notes-sync-with-source but this doesn't prevent the property relating to the link from being saved incorrectly.

akashpal-21 commented 5 days ago

@nobiot Hey sorry for pinging you, but can you comment on this if you have some time. Do you think pushing something like this would be alright?

Thanks for your time.

nobiot commented 4 days ago

Do you think pushing something like this would be alright?

Yes, what you are proposing looks fine. What’s your concern?

I need someone (me included) to repro the issue and test your solution. It is possible that there is a better place to put save-restriction ans widen — I would have thought I’d already placed them but that’s not the case from this issue :)

akashpal-21 commented 4 days ago

@nobiot I have no concern - I am in want for somebody to repro the issue too. Please take your time, let me know if you need any more information if you want to reproduce in the future when you get free time. In my case - it is very trivial to cause this error - the highlight simply needs to be outside the narrow.

Much thanks,

nobiot commented 4 days ago

I cannot narrow an org buffer before highlights are added to the entire buffer because I use "global tracking".

How do you get to this situation?