subtree corruption issues with org-ml-update

[titaniumbones]

trafficstars

Hi,

I'm trying to frame a proper response to your helpful comment in #33, but am running into an issue that looksl ike it might be a bug.

I'm generating a long set of subtrees with this code:

* Fake Assignment forTesting :ASSIGNMENT:
:PROPERTIES:
:ASSIGNMENT_WEIGHT: 10
:ASSIGNMENTID: 735871
:ASSIGNMENT_NAME: Fake Assignment for Testing
:ORG_LMS_ASSIGNMENT_DIRECTORY: fake-assignment-for-testing
:GRADING_TYPE: letter_grade
:IS_QUIZ:  nil
:ORG_LMS_EMAIL_COMMENTS: t
:ORG_LMS_CANVAS_COMMENTS: t
:END:
#+begin_src emacs-lisp
(save-excursion
(forward-line)
(--map (insert
        (format "** TODO Matthew Price %s
:PROPERTIES:
:GRADE:    %s
:CHITS:    0
:NICKNAME: Matthew
:FIRSTNAME: Matthew
:LASTNAME: Price
:MAIL_TO:  [email protected]
:GITHUB:
:ORG_LMS_REPO_BASENAME:
:STUDENTID: 85514
:GRADE_URL: https://q.utoronto.ca/courses/35724/gradebook/speed_grader?assignment_id=735871
:COURSEID: 35724
:BASECOMMIT: none
:ORG_LMS_ASSIGNMENT_DIRECTORY: fake-assignment-for-testing
:MAIL_REPLY: [email protected]
:MAIL_SUBJECT: Comments on Assignment 
:END:
fdsfds %s %s %s

" it it it it it )) (number-sequence 1 100)))
#+end_src

After the trees have been created, I try to modify them in place with:

** Final Subtree
#+begin_src emacs-lisp :results code 
(save-excursion 
  (outline-up-heading 1)
  (let ((items 
         (->>  (org-ml-parse-subtree-at (point))
               (org-ml-match '((:todo-keyword "TODO"))))))
    
    (--map  
     (org-ml-update (lambda (hl)
		      (org-ml-set-property :todo-keyword "READY" hl)) 
it)  items)))
#+end_src

When I run this code, I lose any whitespace at the end of an updated subtree, so that the next subtree pops up into the section of the previous heading. As a result, the structure breaks down. 

I'm assuming this is an issue with mapping over multiple headlings -- I don't see the issue when I replace only one heading at a time. 

Do you see an obvious problem with my code? 

[ndwarshuis]

I think this does what you want:

(save-excursion 
  (outline-up-heading 1)
  (org-ml-update-this-subtree*
    (org-ml-match-map* '((:todo-keyword "TODO"))
      (org-ml-set-property :todo-keyword "READY" it)
      it)))

I'm not sure what caused the whitespace error you were seeing. What you had above should work even if there is a more concise way to write it. Internally your code and my code are calling nearly the same functions.

I'll try and figure this out but for now hopefully this is a valid workaround.

[ndwarshuis]

I at least partially realized what is going on. In your code, you are applying org-ml-update to each headline individually, and every time that function is called the buffer is changing. Since the length of each headline is changing, this means that the next headline in the list is shifting downward in the buffer. However, the values of items in your code has :begin and :end offsets that reflect the buffer state before anything is written, so the offsets will be several characters less than where the next headline moved after the shift.

The solution (which admittedly is not at all obvious) is to reverse the list of nodes (items in your case) before mapping org-ml-update to it. That way any changes in offset will only affect headlines that are already changed.

This isn't the entire story, because by this logic, replacing "READY" in your code with another 4-letter string should still work without reversing. This isn't the case, so something else is also going on...

[titaniumbones]

Lying in bed last night, I wondered if the buffer length changes might be related. I will try reversing the order as soon as I get a chance. But I did also try it with "TODO" and "SENT" to see if that fixed the problem, so as you say, there is likely something else going on too. Does org-element strip whitespace away from the :end properties, and does that possibly affect the update-heading when it rewrites the buffer substring?

[titaniumbones]

yeah, this absolutely does work:

(save-excursion 
  (outline-up-heading 1)
  (let ((items 
         (->>  (org-ml-parse-subtree-at (point))
               (org-ml-match '((:todo-keyword "TODO")))
               (nreverse))))
    
    (--map  
     (org-ml-update (lambda (hl)
		      (org-ml-set-property :todo-keyword "READY" hl)) 
it)  items)))

It also feels faster than before. If that's true, I guess something is triggering more complex calculations in the original version.

[ndwarshuis]

It also sped up for me and I'm guessing that has to do with emacs applying/removing different font faces. In the case of the error, each header that is being partially overwritten is being reformatted as plain org-text and not a headline (which for me are different font faces).

org-ml-update shouldn't be stripping whitespace...in theory.