Ihor Radchenko writes:
> I have executed steps 1-4.
> May you help with backporting Stefan's commit, according to the usual
> process?
Sure. The changes from Emacs's 7c82cc8b975 adapted to Org's current
main (75f3cad71) would be something like the following, but...
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index 4f58a6351..51deee5ec 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -303,6 +303,21 @@ (with-no-warnings ; `replace-buffer-contents' is obsolete
in Emacs 31
(defsubst org-replace-buffer-contents (source &optional max-secs max-costs)
(replace-region-contents (point-min) (point-max) source max-secs
max-costs)))))
+(defalias 'org-replace-region-contents
+ (if (> emacs-major-version 30)
+ #'replace-region-contents
+ ;; The `replace-region-contents' in Emacs<31 does not accept a buffer
+ ;; as SOURCE argument and does not preserve the position well enough.
+ (lambda (beg end source &optional max-secs max-costs)
+ (save-restriction
+ (narrow-to-region beg end)
+ (let ((eobp (eobp)))
+ (with-no-warnings
+ (if (< emacs-major-version 27)
+ (replace-buffer-contents source)
+ (replace-buffer-contents source max-secs max-costs)))
+ (if eobp (goto-char (point-max))))))))
+
(unless (fboundp 'proper-list-p)
;; `proper-list-p' was added in Emacs 27.1. The function below is
;; taken from Emacs subr.el 200195e824b^.
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 2b2dab772..babdc2a3c 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -1427,14 +1427,10 @@ (defun org-edit-src-save ()
;; Temporarily disable read-only features of OVERLAY in order to
;; insert new contents.
(delete-overlay overlay)
- (let ((expecting-bol (save-excursion (goto-char beg) (bolp))))
- (if (version< emacs-version "27.1")
- (progn (delete-region beg end)
- (insert (with-current-buffer write-back-buf
(buffer-string))))
- (save-restriction
- (narrow-to-region beg end)
- (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)
+ (cl-assert (= (point) (+ beg (buffer-size write-back-buf))))
(when (and expecting-bol (not (bolp))) (insert "\n")))
(kill-buffer write-back-buf)
(save-buffer)
@@ -1477,15 +1473,11 @@ (defun org-edit-src-exit ()
;; leading to unexpected results.
;;
https://orgmode.org/list/XF_7mLNCUN8XKtnd7G-NUoAF5Vq0DDafaDdF0v53eFlhQ35N-H3bPA0VkYyDrbEWE-0PEQg8iiyB7NatUtvPEQe6SQyJaTE5vW0CwoUKzqs=@proton.me
(undo-boundary)
- (let ((expecting-bol (save-excursion (goto-char beg) (bolp))))
- (if (version< emacs-version "27.1")
- (progn (delete-region beg end)
- (insert (with-current-buffer write-back-buf
- (buffer-string))))
- (save-restriction
- (narrow-to-region beg end)
- (org-replace-buffer-contents write-back-buf 0.1 nil)
- (goto-char (point-max))))
+ (goto-char beg)
+ (let ((expecting-bol (bolp)))
+ (goto-char end)
+ (org-replace-region-contents beg end write-back-buf 0.1 nil)
+ (cl-assert (= (point) (+ beg (buffer-size write-back-buf))))
(when (and expecting-bol (not (bolp))) (insert "\n")))))
(when write-back-buf (kill-buffer write-back-buf))
;; If we are to return to source buffer, put point at an
--8<---------------cut here---------------end--------------->8---
... as far as I can tell that introduces a regression for the behavior
change discussing up-thread:
Ihor Radchenko writes:
> Kyle Meyer <k...@kyleam.com> writes:
>> For reference, the changes to org-edit-src-save on Emacs's side came in
>> 6bcf41c311b (Org: Use new `replace-region-contents`, 2025-03-29). I was
>> not planning to pull in that commit's changes without adjustments [*],
>> but it may be a good idea to follow that commit's general approach.
>
>> [*] ... at the very least, the changes from Org's 2d9e70b80
>> (org-edit-src-*: When writing back, do not displace point in undo
>> list, 2024-07-09) should be considered. Plus, for compatibility, I
>> think we'd want to keep org-replace-buffer-contents around but mark
>> it as obsolete.
>
> Stefan's commit removes code repetition. As for 2d9e70b80, it does not
> look like Stefan's commit changes anything compared to the existing
> behavior where we do (goto-char (point-max)). Stefan even added an
> assertion, making sure that point is at the right place. So, we should
> be safe I think.
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:
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/org-src.el b/lisp/org-src.el
index babdc2a3c..23c29d12a 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -1427,10 +1427,8 @@ (defun org-edit-src-save ()
;; Temporarily disable read-only features of OVERLAY in order to
;; insert new contents.
(delete-overlay overlay)
- (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))))
(when (and expecting-bol (not (bolp))) (insert "\n")))
(kill-buffer write-back-buf)
(save-buffer)
@@ -1473,11 +1471,8 @@ (defun org-edit-src-exit ()
;; leading to unexpected results.
;;
https://orgmode.org/list/XF_7mLNCUN8XKtnd7G-NUoAF5Vq0DDafaDdF0v53eFlhQ35N-H3bPA0VkYyDrbEWE-0PEQg8iiyB7NatUtvPEQe6SQyJaTE5vW0CwoUKzqs=@proton.me
(undo-boundary)
- (goto-char beg)
- (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))))
(when (and expecting-bol (not (bolp))) (insert "\n")))))
(when write-back-buf (kill-buffer write-back-buf))
;; If we are to return to source buffer, put point at an
--8<---------------cut here---------------end--------------->8---
I'll look into it more this weekend, but of course let me know if you
have any thoughts on that at this point.
