This took me way longer then I want to admit. I learned a lot about the Emacs undo system. Mostly because I overlooked a "(let ((buffer-undo-list t)))".
Anyways here is the new change set. It is layed out in a more logical order, and includes a couple rounds of cleanup. I also added some undo tests. Since the current logic is 1. Remove local variables from buffer 2. User function 3. Add back local variables There is no way to meaningfully and in a stable manner maintain the undo history. So I just squashed all the history together using `org-with-undo-amalgamate'. Tests pass after each patch on emacs 30.2. Tests pass after final patch on emacs 28 and 29. Tests pass after final patch with TZ set to UTC, Europe/Istanbul, and America/New_York. Ihor Radchenko <[email protected]> writes: >> Subject: [PATCH 1/5] Preserve "Local Variables" block without preserving >> following text >> >> * lisp/org-macs.el (org-preserve-local-variables): Extract "Local >> Variables" block exactly to prevent preserving text that follows the >> block. > > Please add Reported-by: and Link: lines to the commit message. > Fixed >> Subject: [PATCH 3/5] Preserve "Local Variables" block when inserting a >> heading >> >> *lisp/org.el (org-insert-heading): When we are respecting content, > > "*lisp..." -> "* lisp..." > > Also, please try the original reproducer and then undo several > times. You will see mess. Fixed
>From 3c666d3d6bb543b15e179796b446863ddfe82e65 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 8 Jan 2026 21:22:44 -0500 Subject: [PATCH 1/9] org-preserve-local-variables: Make macro hygenic * lisp/org-macs.el (org-preserve-local-variables): Use `org-with-gensyms' to avoid defining local variables. --- lisp/org-macs.el | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index c5335ab09..5abb8da7a 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -246,26 +246,27 @@ org-load-noerror-mustsuffix (defmacro org-preserve-local-variables (&rest body) "Execute BODY while preserving local variables." (declare (debug (body))) - `(let ((local-variables - (org-with-wide-buffer - (goto-char (point-max)) - (let ((case-fold-search t)) - (and (re-search-backward "^[ \t]*# +Local Variables:" - (max (- (point) 3000) 1) - t) - (let ((buffer-undo-list t)) - (delete-and-extract-region (point) (point-max))))))) - (tick-counter-before (buffer-modified-tick))) - (unwind-protect (progn ,@body) - (when local-variables - (org-with-wide-buffer - (goto-char (point-max)) - (unless (bolp) (insert "\n")) - (let ((modified (< tick-counter-before (buffer-modified-tick))) - (buffer-undo-list t)) - (insert local-variables) - (unless modified - (restore-buffer-modified-p nil)))))))) + (org-with-gensyms (local-variables tick-counter-before) + `(let ((,local-variables + (org-with-wide-buffer + (goto-char (point-max)) + (let ((case-fold-search t)) + (and (re-search-backward "^[ \t]*# +Local Variables:" + (max (- (point) 3000) 1) + t) + (let ((buffer-undo-list t)) + (delete-and-extract-region (point) (point-max))))))) + (,tick-counter-before (buffer-modified-tick))) + (unwind-protect (progn ,@body) + (when ,local-variables + (org-with-wide-buffer + (goto-char (point-max)) + (unless (bolp) (insert "\n")) + (let ((modified (< ,tick-counter-before (buffer-modified-tick))) + (buffer-undo-list t)) + (insert ,local-variables) + (unless modified + (restore-buffer-modified-p nil))))))))) ;;;###autoload (defmacro org-element-with-disabled-cache (&rest body) base-commit: ce83328b023e988aa8cbb8e8743db35abc24134e -- 2.52.0
>From 4f12b2d4c4a308a1aa95d6b039be5ac5f2758cac Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 11 Jan 2026 16:04:42 -0500 Subject: [PATCH 2/9] test-org/move-subtree: Factor common logic * testing/lisp/test-org.el (test-org/move-subtree): Factor common logic --- testing/lisp/test-org.el | 101 +++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index cd884c02f..29747de5b 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -5888,57 +5888,56 @@ test-org/previous-block (ert-deftest test-org/move-subtree () "Test `org-metaup' and `org-metadown' on headings." - (should - (equal "* H2\n* H1\n" - (org-test-with-temp-text "* H1<point>\n* H2\n" - (org-metadown) - (buffer-string)))) - (should - (equal "* H2\n* H1\n" - (org-test-with-temp-text "* H1\n* H2<point>\n" - (org-metaup) - (buffer-string)))) - (should-error - (org-test-with-temp-text "* H1\n* H2<point>\n" - (org-metadown) - (buffer-string))) - (should-error - (org-test-with-temp-text "* H1<point>\n* H2\n" - (org-metaup) - (buffer-string))) - (should-error - (org-test-with-temp-text "* H1\n** H1.2<point>\n* H2" - (org-metadown) - (buffer-string))) - (should-error - (org-test-with-temp-text "* H1\n** H1.2<point>\n" - (org-metaup) - (buffer-string))) - ;; With selection - (should - (equal "* T\n** H3\n** H1\n** H2\n" - (org-test-with-temp-text "* T\n** <point>H1\n** H2\n** H3\n" - (set-mark (point)) - (search-forward "H2") - (org-metadown) - (buffer-string)))) - (should - (equal "* T\n** H1\n** H2\n** H0\n** H3\n" - (org-test-with-temp-text "* T\n** H0\n** <point>H1\n** H2\n** H3\n" - (set-mark (point)) - (search-forward "H2") - (org-metaup) - (buffer-string)))) - (should-error - (org-test-with-temp-text "* T\n** <point>H1\n** H2\n* T2\n" - (set-mark (point)) - (search-forward "H2") - (org-metadown))) - (should-error - (org-test-with-temp-text "* T\n** <point>H1\n** H2\n* T2\n" - (set-mark (point)) - (search-forward "H2") - (org-metaup)))) + (cl-flet* + ((test-move-subtree (direction + initial-text + &optional expected selection) + (org-test-with-temp-text initial-text + (when selection + (set-mark (point)) + (search-forward selection)) + (cl-ecase direction + (up (org-metaup)) + (down (org-metadown))) + (should (equal expected + (buffer-string)))))) + (test-move-subtree 'down + "* H1<point>\n* H2\n" + "* H2\n* H1\n") + (test-move-subtree 'up + "* H1\n* H2<point>\n" + "* H2\n* H1\n") + (should-error + (test-move-subtree 'down + "* H1\n* H2<point>\n")) + (should-error + (test-move-subtree 'up + "* H1<point>\n* H2\n")) + (should-error + (test-move-subtree 'down + "* H1\n** H1.2<point>\n* H2")) + (should-error + (test-move-subtree 'up + "* H1\n** H1.2<point>\n")) + ;; With selection + (test-move-subtree 'down + "* T\n** <point>H1\n** H2\n** H3\n" + "* T\n** H3\n** H1\n** H2\n" + "H2") + (test-move-subtree 'up + "* T\n** H0\n** <point>H1\n** H2\n** H3\n" + "* T\n** H1\n** H2\n** H0\n** H3\n" + "H2") + (should-error + (test-move-subtree 'down + "* T\n** <point>H1\n** H2\n* T2\n" + nil + "H2")) + (should-error + (test-move-subtree 'up + "* T\n** <point>H1\n** H2\n* T2\n" + nil + "H2")))) (ert-deftest test-org/demote () "Test `org-demote' specifications." -- 2.52.0
>From 774112830b7e43efa8759d4905cb8f37b550cc4a Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 11 Jan 2026 16:13:29 -0500 Subject: [PATCH 3/9] test-org/move-subtree: Test for error more precisely * testing/lisp/test-org.el (test-org/move-subtree): Test for the error in the specific location we expect it to occur (the function call to `org-metaup' or `org-metadown'. Also ensure the error is of type `user-error'. --- testing/lisp/test-org.el | 60 +++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 29747de5b..41a6fb8cc 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -5891,34 +5891,40 @@ test-org/move-subtree (cl-flet* ((test-move-subtree (direction initial-text - &optional expected selection) + expected &optional selection) (org-test-with-temp-text initial-text (when selection (set-mark (point)) (search-forward selection)) - (cl-ecase direction - (up (org-metaup)) - (down (org-metadown))) - (should (equal expected - (buffer-string)))))) + (let ((func + (cl-ecase direction + (up #'org-metaup) + (down #'org-metadown)))) + (if (eq expected 'error) + (should-error + (funcall func) + :type 'user-error) + (funcall func) + (should (equal expected + (buffer-string)))))))) (test-move-subtree 'down "* H1<point>\n* H2\n" "* H2\n* H1\n") (test-move-subtree 'up "* H1\n* H2<point>\n" "* H2\n* H1\n") - (should-error - (test-move-subtree 'down - "* H1\n* H2<point>\n")) - (should-error - (test-move-subtree 'up - "* H1<point>\n* H2\n")) - (should-error - (test-move-subtree 'down - "* H1\n** H1.2<point>\n* H2")) - (should-error - (test-move-subtree 'up - "* H1\n** H1.2<point>\n")) + (test-move-subtree 'down + "* H1\n* H2<point>\n" + 'error) + (test-move-subtree 'up + "* H1<point>\n* H2\n" + 'error) + (test-move-subtree 'down + "* H1\n** H1.2<point>\n* H2" + 'error) + (test-move-subtree 'up + "* H1\n** H1.2<point>\n" + 'error) ;; With selection (test-move-subtree 'down "* T\n** <point>H1\n** H2\n** H3\n" @@ -5928,16 +5934,14 @@ test-org/move-subtree "* T\n** H0\n** <point>H1\n** H2\n** H3\n" "* T\n** H1\n** H2\n** H0\n** H3\n" "H2") - (should-error - (test-move-subtree 'down - "* T\n** <point>H1\n** H2\n* T2\n" - nil - "H2")) - (should-error - (test-move-subtree 'up - "* T\n** <point>H1\n** H2\n* T2\n" - nil - "H2")))) + (test-move-subtree 'down + "* T\n** <point>H1\n** H2\n* T2\n" + 'error + "H2") + (test-move-subtree 'up + "* T\n** <point>H1\n** H2\n* T2\n" + 'error + "H2"))) (ert-deftest test-org/demote () "Test `org-demote' specifications." -- 2.52.0
>From 67785fd8c6f8a63ef212d037edeef920715a99b3 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 11 Jan 2026 16:33:39 -0500 Subject: [PATCH 4/9] test-org/move-subtree: Test that undo works as expected * testing/lisp/test-org.el (test-org/move-subtree): Test that undo works as expected. --- testing/lisp/test-org.el | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 41a6fb8cc..bd9ce6108 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -5887,12 +5887,14 @@ test-org/previous-block ;;; Outline structure (ert-deftest test-org/move-subtree () - "Test `org-metaup' and `org-metadown' on headings." + "Test `org-metaup' and `org-metadown' on headings. +Also ensure undo works as expected." (cl-flet* ((test-move-subtree (direction initial-text expected &optional selection) (org-test-with-temp-text initial-text + (buffer-enable-undo) (when selection (set-mark (point)) (search-forward selection)) @@ -5906,6 +5908,11 @@ test-org/move-subtree :type 'user-error) (funcall func) (should (equal expected + (buffer-string))) + (deactivate-mark) + (undo-boundary) + (undo) + (should (equal (string-replace "<point>" "" initial-text) (buffer-string)))))))) (test-move-subtree 'down "* H1<point>\n* H2\n" -- 2.52.0
>From a9f5fa1559f3abc4af9e9fcd27f767425f2d0b8a Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Sun, 11 Jan 2026 20:54:09 -0500 Subject: [PATCH 5/9] org-preserve-local-variables: Amalgamate undo * lisp/org-macs.el (org-preserve-local-variables): Amalgamate undo as undoing in steps could leave the buffer in a state the user has never seen before. --- lisp/org-macs.el | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index 5abb8da7a..aabaf13d9 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -247,26 +247,25 @@ org-preserve-local-variables "Execute BODY while preserving local variables." (declare (debug (body))) (org-with-gensyms (local-variables tick-counter-before) - `(let ((,local-variables - (org-with-wide-buffer - (goto-char (point-max)) - (let ((case-fold-search t)) - (and (re-search-backward "^[ \t]*# +Local Variables:" - (max (- (point) 3000) 1) - t) - (let ((buffer-undo-list t)) - (delete-and-extract-region (point) (point-max))))))) - (,tick-counter-before (buffer-modified-tick))) - (unwind-protect (progn ,@body) - (when ,local-variables - (org-with-wide-buffer - (goto-char (point-max)) - (unless (bolp) (insert "\n")) - (let ((modified (< ,tick-counter-before (buffer-modified-tick))) - (buffer-undo-list t)) - (insert ,local-variables) - (unless modified - (restore-buffer-modified-p nil))))))))) + `(org-with-undo-amalgamate + (let ((,local-variables + (org-with-wide-buffer + (goto-char (point-max)) + (let ((case-fold-search t)) + (and (re-search-backward "^[ \t]*# +Local Variables:" + (max (- (point) 3000) 1) + t) + (delete-and-extract-region (point) (point-max)))))) + (,tick-counter-before (buffer-modified-tick))) + (unwind-protect (progn ,@body) + (when ,local-variables + (org-with-wide-buffer + (goto-char (point-max)) + (unless (bolp) (insert "\n")) + (let ((modified (< ,tick-counter-before (buffer-modified-tick)))) + (insert ,local-variables) + (unless modified + (restore-buffer-modified-p nil)))))))))) ;;;###autoload (defmacro org-element-with-disabled-cache (&rest body) -- 2.52.0
>From 8de2ea252efb0e294de39099a16aca1b11f20780 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 4 Dec 2025 12:03:27 -0500 Subject: [PATCH 6/9] Preserve "Local Variables" block without preserving following text * lisp/org-macs.el (org-preserve-local-variables): Extract "Local Variables" block exactly to prevent preserving text that follows the block. Reported-by: Rens Oliemans <[email protected]> Link: https://list.orgmode.org/[email protected] --- lisp/org-macs.el | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index aabaf13d9..74d7902ba 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -252,18 +252,29 @@ org-preserve-local-variables (org-with-wide-buffer (goto-char (point-max)) (let ((case-fold-search t)) - (and (re-search-backward "^[ \t]*# +Local Variables:" - (max (- (point) 3000) 1) - t) - (delete-and-extract-region (point) (point-max)))))) + (and (re-search-backward + ,(rx-let ((prefix + (seq line-start (zero-or-more whitespace) + "#" (one-or-more whitespace)))) + (rx prefix "Local Variables:" + (one-or-more anychar) + prefix "End:" + (zero-or-more whitespace) (optional "\n"))) + (max (- (point) 3000) 1) + t) + (cons (match-beginning 0) + (delete-and-extract-region (match-beginning 0) + (match-end 0))))))) (,tick-counter-before (buffer-modified-tick))) (unwind-protect (progn ,@body) (when ,local-variables (org-with-wide-buffer - (goto-char (point-max)) - (unless (bolp) (insert "\n")) (let ((modified (< ,tick-counter-before (buffer-modified-tick)))) - (insert ,local-variables) + (if (not modified) + (goto-char (car ,local-variables)) + (goto-char (point-max)) + (unless (bolp) (insert "\n"))) + (insert (cdr ,local-variables)) (unless modified (restore-buffer-modified-p nil)))))))))) -- 2.52.0
>From 89668c3dc6fe41383132eb47b891c2f08b3afccd Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 4 Dec 2025 12:05:35 -0500 Subject: [PATCH 7/9] Testing: Test moving a subtree with a "Local Variables" block * testing/lisp/test-org.el (test-org/move-subtree): Add tests that include a "Local Variables" block. --- testing/lisp/test-org.el | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index bd9ce6108..b4bd961d0 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -5932,6 +5932,16 @@ test-org/move-subtree (test-move-subtree 'up "* H1\n** H1.2<point>\n" 'error) + ;; Local variables + (let ((local-variable-string "# Local Variables: +# fill-column: 120 +# End:\n")) + (test-move-subtree 'down + (concat "* H1<point>\n* H2\n" local-variable-string) + (concat "* H2\n* H1\n" local-variable-string)) + (test-move-subtree 'down + (concat "* H1<point>\n* H2\n" local-variable-string "* H3\n") + (concat "* H2\n* H1\n* H3\n" local-variable-string))) ;; With selection (test-move-subtree 'down "* T\n** <point>H1\n** H2\n** H3\n" -- 2.52.0
>From a95830c10def91f5a5292c947395d51964d01956 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 4 Dec 2025 12:06:20 -0500 Subject: [PATCH 8/9] Preserve "Local Variables" block when inserting a heading * lisp/org.el (org-insert-heading): When we are respecting content, preserve the "Local Variables" block by using `org-preserve-local-variables'. Reported-by: Rens Oliemans <[email protected]> Link: https://list.orgmode.org/[email protected] --- lisp/org.el | 77 +++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index e3d0073c2..75ee994d3 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -6637,44 +6637,45 @@ org-insert-heading (invisible-p (max (1- (point)) (point-min))))) ;; Position point at the location of insertion. Make sure we ;; end up on a visible headline if INVISIBLE-OK is nil. - (org-with-limited-levels - (if (not current-level) (outline-next-heading) ;before first headline - (org-back-to-heading invisible-ok) - (when (equal arg '(16)) (org-up-heading-safe)) - (org-end-of-subtree invisible-ok 'to-heading))) - ;; At `point-max', if the file does not have ending newline, - ;; create one, so that we are not appending stars at non-empty - ;; line. - (unless (bolp) (insert "\n")) - (when (and blank? (save-excursion - (backward-char) - (org-before-first-heading-p))) - (insert "\n") - (backward-char)) - (when (and (not current-level) (not (eobp)) (not (bobp))) - (when (org-at-heading-p) (insert "\n")) - (backward-char)) - (unless (and blank? (org-previous-line-empty-p)) - (org-N-empty-lines-before-current (if blank? 1 0))) - (insert stars " " "\n") - ;; Move point after stars. - (backward-char) - ;; Retain blank lines before next heading. - (funcall maybe-add-blank-after blank?) - ;; When INVISIBLE-OK is non-nil, ensure newly created headline - ;; is visible. - (unless invisible-ok - (if (eq org-fold-core-style 'text-properties) - (cond - ((org-fold-folded-p - (max (point-min) - (1- (line-beginning-position)))) - (org-fold-region (line-end-position 0) (line-end-position) nil)) - (t nil)) - (pcase (get-char-property-and-overlay (point) 'invisible) - (`(outline . ,o) - (move-overlay o (overlay-start o) (line-end-position 0))) - (_ nil))))) + (org-preserve-local-variables + (org-with-limited-levels + (if (not current-level) (outline-next-heading) ;before first headline + (org-back-to-heading invisible-ok) + (when (equal arg '(16)) (org-up-heading-safe)) + (org-end-of-subtree invisible-ok 'to-heading))) + ;; At `point-max', if the file does not have ending newline, + ;; create one, so that we are not appending stars at non-empty + ;; line. + (unless (bolp) (insert "\n")) + (when (and blank? (save-excursion + (backward-char) + (org-before-first-heading-p))) + (insert "\n") + (backward-char)) + (when (and (not current-level) (not (eobp)) (not (bobp))) + (when (org-at-heading-p) (insert "\n")) + (backward-char)) + (unless (and blank? (org-previous-line-empty-p)) + (org-N-empty-lines-before-current (if blank? 1 0))) + (insert stars " " "\n") + ;; Move point after stars. + (backward-char) + ;; Retain blank lines before next heading. + (funcall maybe-add-blank-after blank?) + ;; When INVISIBLE-OK is non-nil, ensure newly created headline + ;; is visible. + (unless invisible-ok + (if (eq org-fold-core-style 'text-properties) + (cond + ((org-fold-folded-p + (max (point-min) + (1- (line-beginning-position)))) + (org-fold-region (line-end-position 0) (line-end-position) nil)) + (t nil)) + (pcase (get-char-property-and-overlay (point) 'invisible) + (`(outline . ,o) + (move-overlay o (overlay-start o) (line-end-position 0))) + (_ nil)))))) ;; At a headline... ((org-at-heading-p) (cond ((bolp) -- 2.52.0
>From 08991292a808b7981d1bc8435898b60e1dc26ee7 Mon Sep 17 00:00:00 2001 From: Morgan Smith <[email protected]> Date: Thu, 4 Dec 2025 12:06:44 -0500 Subject: [PATCH 9/9] Testing: Test inserting a heading with a "Local Variables" block * testing/lisp/test-org.el (test-org/insert-heading): Test inserting a heading with a "Local Variables" block. --- testing/lisp/test-org.el | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index b4bd961d0..771721035 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -1889,6 +1889,20 @@ test-org/insert-heading (org-test-with-temp-text "<point>P" (org-insert-heading) (buffer-string)))) + ;; Move local variable string to end when we respect the content + (let ((local-variable-string "# Local Variables: +# fill-column: 120 +# End:\n")) + (should + (equal (concat "* \n" local-variable-string) + (org-test-with-temp-text local-variable-string + (org-insert-heading-respect-content) + (buffer-string)))) + (should + (equal (concat "* H\n* \n" local-variable-string) + (org-test-with-temp-text (concat "* H<point>\n" local-variable-string) + (org-insert-heading-respect-content) + (buffer-string))))) ;; In the middle of a line, split the line if allowed, otherwise, ;; insert the headline at its end. (should -- 2.52.0
