Kyle Meyer <k...@kyleam.com> writes:
> - (org-replace-buffer-contents write-back-buf 0.1 nil)
> - (goto-char (point-max))))
> + (let ((expecting-bol (bolp)))
> + (goto-char end)
> + (org-replace-region-contents beg end write-back-buf 0.1 nil)
>
> ... as far as I can tell that introduces a regression for the behavior
> change discussing up-thread:
Hmm. The only difference I am seeing here is that we change the undo
record order in `buffer-undo-list'. Before: (1) replace region and
record edit in the undo-list; (2) move point and record movement in the
undo-list. After: (1) move point and record movement; (2) replace region
and record edit.
That might indeed change the undo behavior.
> If I apply the above changes, the test case from the message referenced
> by 2d9e70b80 changes behavior, moving point to the end of the block
> following an undo.
>
> So perhaps we need something like this on top:
> - (let ((expecting-bol (bolp)))
> - (goto-char end)
> + (let ((expecting-bol (save-excursion (goto-char beg) (bolp))))
> (org-replace-region-contents beg end write-back-buf 0.1 nil)
> - (cl-assert (= (point) (+ beg (buffer-size write-back-buf))))
After what you found I have doubts. In particular, I have doubts about
compat version of `org-replace-region-contents'.
+(defalias 'org-replace-region-contents
+ (if (> emacs-major-version 30)
+ #'replace-region-contents
....
+ (if (< emacs-major-version 27)
+ (replace-buffer-contents source)
+ (replace-buffer-contents source max-secs max-costs)))
+ (if eobp (goto-char (point-max))))))))
In Emacs 30 and older, we will record two points in undo history vs. a
single point in Emacs 31+ (because of `goto-char'). This is a subtle
difference that matters for us.
Conclusion: let's not touch the working code just for the sake of
refactoring. It is way too brittle.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
