Re: Reconciling org-mode idiosyncrasies with Emacs core

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

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

Hi Nicolas,

I took a stab at making RET obey electric-indent-mode in org-mode.  I've
got something working; I'd like to ask for a review before moving on to
Changelog and ORG-NEWS entries (and tackling C-j… and maybe writing a
few unit tests?).

Here's the patch, with some additional comments below:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-return.patch --]
[-- Type: text/x-patch, Size: 2515 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index e82463046..681328d96 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17644,20 +17644,32 @@ 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
@@ -17708,23 +17720,20 @@ 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.

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


Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
>
>> Do you think a patch that
>>
>> - tweaked org-return (bound to RET) to default its INDENT argument to
>>   the current value of electric-indent-mode,

After taking an in-depth look at 'org-return' and 'newline', I decided
to "let the knife do the work" and simply keep calling 'newline', though
with additional arguments:

- INTERACTIVE is what makes 'newline' run 'post-self-insert-hook' (thus
  triggering indentation through electric-indent-mode),

- ARG wasn't strictly necessary, but it seemed harmless to add it, and
  it allows inserting multiple newlines, thus removing one more "Org
  idiosyncrasy".

I felt that introducing org--newline made the code clearer, but I can
understand if it seems too trivial to keep.  I took the liberty of using
this function in the "list item" case too, otherwise there's no way to
indent the trailing text.


> The change will not appear overnight in Org, i.e., not in Org stable's
> branch (Org 9.3.X), and it will be announced in ORG-NEWS.

I'll work on ORG-NEWS (plus Changelog entries, plus unit tests) as soon
as I'm confident that my approach is satisfactory.

(Out of curiosity, could it be argued that this is solving a "bug" in
org-mode and, as such, could be committed to Emacs core first, then
backported to the org-mode repository?  I don't feel strongly either
way, I wouldn't want to make things more complicated for Org
maintainers.)


Now for C-j, in order to minimize breakage (for anyone calling
org-return-indent from Lisp code) and simplify disabling the new
behaviour (by simply turning off electric-indent-mode in Org), should we
bind C-j to a new function?  E.g.:

(defun org-return-and-maybe-indent ()
  (interactive)
  (org-return (not electric-indent-mode)))


Thank you for your time.