[PATCH] Make RET and C-j obey `electric-indent-mode' in org-mode (was: Reconciling org-mode idiosyncrasies with Emacs core)

["Kévin Le Gouguec" <kevin.legouguec@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 130 bytes --]

Hello folks,

Here's a complete patch to make RET and C-j honor electric-indent-mode
in org-mode, targeting Org's master branch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-RET-and-C-j-obey-electric-indent-mode.patch --]
[-- Type: text/x-patch, Size: 12251 bytes --]

From ec3b06f02aa875b3c78b076e846081ce4560ec18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Tue, 5 May 2020 19:01:07 +0200
Subject: [PATCH] Make RET and C-j obey `electric-indent-mode'

* etc/ORG-NEWS: Announce the change.

* lisp/org-compat.el (org-return-indent): Deprecate this command.

* lisp/org-keys.el (org-mode-map): Rebind C-j to a command emulating
`electric-newline-and-maybe-indent'.

* lisp/org.el (org-cdlatex-environment-indent): Stop using the now
obsolete function.
(org--newline): New helper function.
(org-return): Use it to transparently handle `electric-indent-mode'.
(org-return-and-maybe-indent): New command to emulate
`electric-newline-and-maybe-indent' while taking care of Org special
cases (tables, links, timestamps).

* testing/lisp/test-org.el (test-org/with-electric-indent,
test-org/without-electric-indent): New tests.

* testing/org-test.el (org-test-with-minor-mode): New helper to set a
minor mode to a specific state, and reset it afterward.
---
 etc/ORG-NEWS             | 33 +++++++++++++++++
 lisp/org-compat.el       |  9 +++++
 lisp/org-keys.el         |  2 +-
 lisp/org.el              | 43 ++++++++++++++---------
 testing/lisp/test-org.el | 76 ++++++++++++++++++++++++++++++++++++++++
 testing/org-test.el      |  9 +++++
 6 files changed, 155 insertions(+), 17 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c10e82f53..9c7e0d604 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -215,6 +215,32 @@ configured for ClojureScript will /not/ work.
 Babel Java blocks recognize header argument =:cmdargs= and pass its
 value in call to =java=.
 
+*** =RET= and =C-j= now obey ~electric-indent-mode~
+
+Since Emacs 24.4, ~electric-indent-mode~ is enabled by default.  In
+most major modes, this causes =RET= to reindent the current line and
+indent the new line, and =C-j= to insert a newline without indenting.
+
+Org-mode now obeys this minor mode: when ~electric-indent-mode~ is
+enabled, and point is neither in a table nor on a timestamp or a link:
+
+- =RET= (bound to ~org-return~) reindents the current line and indents
+  the new line;
+- =C-j= (bound to the new command ~org-return-and-maybe-indent~)
+  merely inserts a newline.
+
+To get the previous behaviour back, disable ~electric-indent-mode~
+explicitly:
+
+#+begin_src emacs-lisp
+(add-hook 'org-mode-hook (lambda () (electric-indent-mode -1)))
+#+end_src
+
+*** New optional numeric argument for ~org-return~
+
+In situations where ~org-return~ calls ~newline~, multiple newlines
+can now be inserted with this prefix argument.
+
 ** New commands
 *** ~org-table-header-line-mode~
 
@@ -303,6 +329,13 @@ Use ~org-hide-block-toggle~ instead.
 This function was not used in the code base, and has no clear use
 either.  It has been marked for future removal.  Please contact the
 mailing list if you use this function.
+
+*** Deprecated ~org-return-indent~ function
+
+In Elisp code, use ~(org-return t)~ instead.  Interactively, =C-j= is
+now bound to ~org-return-and-maybe-indent~, which indents the new line
+when ~electric-indent-mode~ is disabled.
+
 *** Removed ~org-maybe-keyword-time-regexp~
 
 The variable was not used in the code base.
diff --git a/lisp/org-compat.el b/lisp/org-compat.el
index aae8efbd3..2b35535fa 100644
--- a/lisp/org-compat.el
+++ b/lisp/org-compat.el
@@ -678,6 +678,15 @@ an error.  Return a non-nil value when toggling is successful."
             (goto-char (match-beginning 0))
             (org-hide-block-toggle)))))))
 
+(defun org-return-indent ()
+  "Goto next table row or insert a newline and indent.
+Calls `org-table-next-row' or `newline-and-indent', depending on
+context.  See the individual commands for more information."
+  (declare (obsolete "use `org-return' with INDENT set to t instead."
+		     "Org 9.4"))
+  (interactive)
+  (org-return t))
+
 (defmacro org-with-silent-modifications (&rest body)
   (declare (obsolete "use `with-silent-modifications' instead." "Org 9.2")
 	   (debug (body)))
diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index d358da763..7c0cc9216 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -618,7 +618,7 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 (org-defkey org-mode-map (kbd "C-c C-k") #'org-kill-note-or-show-branches)
 (org-defkey org-mode-map (kbd "C-c #") #'org-update-statistics-cookies)
 (org-defkey org-mode-map (kbd "RET") #'org-return)
-(org-defkey org-mode-map (kbd "C-j") #'org-return-indent)
+(org-defkey org-mode-map (kbd "C-j") #'org-return-and-maybe-indent)
 (org-defkey org-mode-map (kbd "C-c ?") #'org-table-field-info)
 (org-defkey org-mode-map (kbd "C-c SPC") #'org-table-blank-field)
 (org-defkey org-mode-map (kbd "C-c +") #'org-table-sum)
diff --git a/lisp/org.el b/lisp/org.el
index 63de7306c..dbd072aff 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -15580,7 +15580,7 @@ environment remains unintended."
       ;; Get indentation of next line unless at column 0.
       (let ((ind (if (bolp) 0
 		   (save-excursion
-		     (org-return-indent)
+		     (org-return t)
 		     (prog1 (current-indentation)
 		       (when (progn (skip-chars-forward " \t") (eolp))
 			 (delete-region beg (point)))))))
@@ -17647,20 +17647,31 @@ call `open-line' on the very first character."
       (org-table-insert-row)
     (open-line n)))
 
-(defun org-return (&optional indent)
+(defun org--newline (indent arg interactive)
+  "Call `newline-and-indent' or just `newline'.
+If INDENT is non-nil, call `newline-and-indent' with ARG to
+indent unconditionally; otherwise, call `newline' with ARG and
+INTERACTIVE, which can trigger indentation if
+`electric-indent-mode' is enabled."
+  (if indent
+      (newline-and-indent arg)
+    (newline arg interactive)))
+
+(defun org-return (&optional indent arg interactive)
   "Goto next table row or insert a newline.
 
 Calls `org-table-next-row' or `newline', depending on context.
 
 When optional INDENT argument is non-nil, call
-`newline-and-indent' instead of `newline'.
+`newline-and-indent' with ARG, otherwise call `newline' with ARG
+and INTERACTIVE.
 
 When `org-return-follows-link' is non-nil and point is on
 a timestamp or a link, call `org-open-at-point'.  However, it
 will not happen if point is in a table or on a \"dead\"
 object (e.g., within a comment).  In these case, you need to use
 `org-open-at-point' directly."
-  (interactive)
+  (interactive "*i\nP\np")
   (let ((context (if org-return-follows-link (org-element-context)
 		   (org-element-at-point))))
     (cond
@@ -17711,30 +17722,30 @@ object (e.g., within a comment).  In these case, you need to use
 	 (t (org--align-tags-here tags-column))) ;preserve tags column
 	(end-of-line)
 	(org-show-entry)
-	(if indent (newline-and-indent) (newline))
+	(org--newline indent arg interactive)
 	(when string (save-excursion (insert (org-trim string))))))
      ;; In a list, make sure indenting keeps trailing text within.
-     ((and indent
-	   (not (eolp))
+     ((and (not (eolp))
 	   (org-element-lineage context '(item)))
       (let ((trailing-data
 	     (delete-and-extract-region (point) (line-end-position))))
-	(newline-and-indent)
+	(org--newline indent arg interactive)
 	(save-excursion (insert trailing-data))))
      (t
       ;; Do not auto-fill when point is in an Org property drawer.
       (let ((auto-fill-function (and (not (org-at-property-p))
 				     auto-fill-function)))
-	(if indent
-	    (newline-and-indent)
-	  (newline)))))))
+	(org--newline indent arg interactive))))))
 
-(defun org-return-indent ()
-  "Goto next table row or insert a newline and indent.
-Calls `org-table-next-row' or `newline-and-indent', depending on
-context.  See the individual commands for more information."
+(defun org-return-and-maybe-indent ()
+  "Goto next table row, or insert a newline.
+Call `org-table-next-row' or `org-return', depending on context.
+See the individual commands for more information.
+
+When inserting a newline, indent the new line if
+`electric-indent-mode' is disabled."
   (interactive)
-  (org-return t))
+  (org-return (not electric-indent-mode)))
 
 (defun org-ctrl-c-tab (&optional arg)
   "Toggle columns width in a table, or show children.
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 2c78e1e23..d481d28e3 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -1310,6 +1310,82 @@
 	    (org-return)
 	    (buffer-string)))))
 
+(ert-deftest test-org/with-electric-indent ()
+  "Test RET and C-j specifications with `electric-indent-mode' on."
+  ;; Call commands interactively, since this is how `newline' knows it
+  ;; must run `post-self-insert-hook'.
+  (org-test-with-minor-mode electric-indent-mode t
+    ;; RET, like `newline', should indent.
+    (should
+     (equal "  Para\n  graph"
+	    (org-test-with-temp-text "  Para<point>graph"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    (should
+     (equal "- item1\n  item2"
+	    (org-test-with-temp-text "- item1<point>item2"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    (should
+     (equal "* heading\n  body"
+	    (org-test-with-temp-text "* heading<point>body"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    ;; C-j, like `electric-newline-and-maybe-indent', should not indent.
+    (should
+     (equal "  Para\ngraph"
+	    (org-test-with-temp-text "  Para<point>graph"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))
+    (should
+     (equal "- item1\nitem2"
+	    (org-test-with-temp-text "- item1<point>item2"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))
+    (should
+     (equal "* heading\nbody"
+	    (org-test-with-temp-text "* heading<point>body"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))))
+
+(ert-deftest test-org/without-electric-indent ()
+  "Test RET and C-j specifications with `electric-indent-mode' off."
+  ;; Call commands interactively, since this is how `newline' knows it
+  ;; must run `post-self-insert-hook'.
+  (org-test-with-minor-mode electric-indent-mode nil
+    ;; RET, like `newline', should not indent.
+    (should
+     (equal "  Para\ngraph"
+	    (org-test-with-temp-text "  Para<point>graph"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    (should
+     (equal "- item1\nitem2"
+	    (org-test-with-temp-text "- item1<point>item2"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    (should
+     (equal "* heading\nbody"
+	    (org-test-with-temp-text "* heading<point>body"
+	      (call-interactively 'org-return)
+	      (buffer-string))))
+    ;; C-j, like `electric-newline-and-maybe-indent', should indent.
+    (should
+     (equal "  Para\n  graph"
+	    (org-test-with-temp-text "  Para<point>graph"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))
+    (should
+     (equal "- item1\n  item2"
+	    (org-test-with-temp-text "- item1<point>item2"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))
+    (should
+     (equal "* heading\n  body"
+	    (org-test-with-temp-text "* heading<point>body"
+	      (call-interactively 'org-return-and-maybe-indent)
+	      (buffer-string))))))
+
 (ert-deftest test-org/meta-return ()
   "Test M-RET (`org-meta-return') specifications."
   ;; In a table field insert a row above.
diff --git a/testing/org-test.el b/testing/org-test.el
index 6904e16d1..69cdb905b 100644
--- a/testing/org-test.el
+++ b/testing/org-test.el
@@ -230,6 +230,15 @@ point at the beginning of the buffer."
 	 (delete-file file)))))
 (def-edebug-spec org-test-with-temp-text-in-file (form body))
 
+(defmacro org-test-with-minor-mode (mode state &rest body)
+  "Run BODY after setting MODE to STATE.
+Restore MODE to its former state afterward."
+  (declare (debug (sexp sexp body)) (indent 2))
+  `(let ((old-state ,mode))
+       (,mode (if ,state 1 0))
+       ,@body
+       (,mode (if old-state 1 0))))
+
 (defun org-test-table-target-expect (target &optional expect laps
 &rest tblfm)
   "For all TBLFM: Apply the formula to TARGET, compare EXPECT with result.
-- 
2.26.2


[-- Attachment #3: Type: text/plain, Size: 1319 bytes --]


To summarize (see also the patch's changelog message, and the ORG-NEWS
entries which explain how to restore the current behaviour):

- `org-return' (bound to RET) gains new arguments, and an interactive
  spec matching `newline'.

- In situations where `org-return' must insert a newline,

    - if the INDENT argument is non-nil, `newline-and-indent' is called
      unconditionally, as before,

    - otherwise, `newline' is called with ARG and INTERACTIVE; if
      electric-indent-mode is on and INTERACTIVE is non-nil, this will
      trigger indentation.

- `org-return-indent', the previous binding for C-j, is deprecated.

- C-j is now bound to `org-return-and-maybe-indent', which calls
  (org-return t) when electric-indent-mode is disabled, and
  (org-return nil) otherwise.

- In addition to the new unit tests, there is a new test helper to
  temporarily set or unset a minor mode.


Let me know if I messed up something, or if there are things to tweak
here and there.  org-test-with-minor-mode might be overkill (and I am
not used to writing macros, let alone edebug specs, so I might have
goofed somewhere), but it does make the tests easier to read & write,
so… 🤷

(I'm surprised such a macro does not already exist; did I miss it?)


Thank you for your time.