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

[akashpal-21]

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 when condition 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]

(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]

@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]

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]

@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]

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?

[akashpal-21]

@nobiot Hello, sorry I got busy with responsibilities and couldn't get back to this then.
I will try to explain it - but sometimes I don't know how to phrase it, language is not my forte.

Lets say we are working on a buffer and we have remarks here and there - but now we narrow to a headline - we work there, and save - without widening - the current implementation causes remarks outside the narrowed region to lose track of their position markers.

Does it make sense?

The problem arises when we save from a narrowed situation and remarks are out of the range of the narrow. It should be trivial to reproduce the problem.

Go to a buffer where there exists remarks
Choose a headline to narrow into
Save here.

[akashpal-21]

untitled.mp4

Attaching a screen record to show the issue - the problem is very simple..

[akashpal-21]

@nobiot Interestingly - the position markers are being correctly being registered now that I test it again - I almost feel stupid now. I swear they were reporting nil before, but now that I test them, they record the delta correctly - the error is normal, but my issue was that the position markers

:org-remark-beg: 51
:org-remark-end: 55
:org-remark-link: [[file:~/Desktop/test1/target.org::14]]

Were not being recorded correctly.

I have no idea how it got resolved automatically?

Currently nothing seems to be needed to be done. Will be closing this issue unless I can reproduce the issue of the markers being misrepresented again. Sorry.. ?

[akashpal-21]

Okay I have caused it again - but it seems only the :org-remark-link: gets wrongly referenced - namely the line number jumps to the headline of the current narrowed region. That is all that seems to be the issue - the position markers are otherwise correctly registered

untitled.mp4

Focus on the :org-remark-link: when I am saving -

[nobiot]

Thanks for additional detail... I will look, but let me have some time. On vacation in a remote place :)

[nobiot]

Looks like I should have used a PR to send this commit d37ec06 but it is meant to fix the issue of :org-remark-link: gets wrongly referenced.

Thank you @akashpal-21 for staying with me on this one. I believe it's fixed. Please open it again if it is not with the commit.

[akashpal-21]

Much thanks for the fix Nobiot :)

[akashpal-21]

Got time to update today, fix confirmed for both the problems, thanks.