Re: [PATCH] Fix org-comment-line-break-function

[Ihor Radchenko <yantar92@gmail.com>]

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

Kaushal Modi <kaushal.modi@gmail.com> writes:

> Hi Nicolas,
>
> I have added few tests in the updated patch pasted in this email.
> I have made the tests for (call-interactive #'default-indent-new-line)
> because that the interactive function M-j is bound to by default.
>
> Can you please review and commit it? The machine I am on right now does not
> allow external ssh access.

Unfortunately, we cannot rely on the built-in `comment-indent-new-line'
to fill Org comments. This is because Emacs uses a complex entanglement
of regexp heuristics to determine comment at point and its boundaries.

I am attaching an alternative patch to fix the issue using Org element
parser. Note that I reused your tests.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-comment-line-break-function-Avoid-built-in-Emacs.patch --]
[-- Type: text/x-patch, Size: 3416 bytes --]

From ded35b55ca694e3eb831878160ac37ceec48b08e Mon Sep 17 00:00:00 2001
Message-Id: <ded35b55ca694e3eb831878160ac37ceec48b08e.1664428038.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Thu, 29 Sep 2022 13:02:46 +0800
Subject: [PATCH] org-comment-line-break-function: Avoid built-in Emacs comment
 machinery

* lisp/org.el (org-comment-line-break-function): Rely on Org
parser (`org-adaptive-fill-function') to determine comment filling.
Handle nil values of `fill-prefix' correctly.

* testing/lisp/test-org.el (test-org/default-indent-new-line): New
test.  Written by Kaushal Modi <kaushal.modi@gmail.com>
https://list.orgmode.org/CAFyQvY36DkBSNy2mPxDNZWeoTjUK8mAqgJM-zHxNamfReqGkuQ@mail.gmail.com/

Reported-by: Richard Lawrence <richard.lawrence@uni-tuebingen.de>
Link: https://list.orgmode.org/87lf18fue9.fsf@aquinas.i-did-not-set--mail-host-address--so-tickle-me/
---
 lisp/org.el              | 18 ++++++++++++------
 testing/lisp/test-org.el | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 036384a04..5ff60baf6 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19295,12 +19295,18 @@ (defun org-comment-line-break-function (&optional soft)
   "Break line at point and indent, continuing comment if within one.
 The inserted newline is marked hard if variable
 `use-hard-newlines' is true, unless optional argument SOFT is
-non-nil."
-  (if soft (insert-and-inherit ?\n) (newline 1))
-  (save-excursion (forward-char -1) (delete-horizontal-space))
-  (delete-horizontal-space)
-  (indent-to-left-margin)
-  (insert-before-markers-and-inherit fill-prefix))
+non-nil.
+
+This function is a simplified version of `comment-indent-new-line'
+that bypasses the complex Emacs machinery dealing with comments.
+We instead rely on Org parser, utilizing `org-adaptive-fill-function'"
+  (let ((fill-prefix (org-adaptive-fill-function)))
+    (if soft (insert-and-inherit ?\n) (newline 1))
+    (save-excursion (forward-char -1) (delete-horizontal-space))
+    (delete-horizontal-space)
+    (indent-to-left-margin)
+    (when fill-prefix
+      (insert-before-markers-and-inherit fill-prefix))))
 
 
 ;;; Fixed Width Areas
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 1b4157d0e..4a6a3a0b0 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -1405,6 +1405,26 @@ (ert-deftest test-org/indent-region ()
 	    (org-indent-region (point-min) (point-max))
 	    (buffer-string)))))
 
+(ert-deftest test-org/default-indent-new-line ()
+  "Test behavior of default binding `M-j'."
+  ;; Calling `M-j' when point is not in an Org comment:
+  (should
+   (equal "* Some heading\n"
+  (org-test-with-temp-text "* Some heading<point>"
+                           (call-interactively #'default-indent-new-line)
+                           (buffer-string))))
+  ;; Calling `M-j' when point is in an Org comment:
+  (should
+   (equal "# Some Org comment\n# "
+  (org-test-with-temp-text "# Some Org comment<point>"
+                           (call-interactively #'default-indent-new-line)
+                           (buffer-string))))
+  (should
+   (equal "# Some Org\n# comment"
+  (org-test-with-temp-text "# Some Org <point>comment"
+                           (call-interactively #'default-indent-new-line)
+                           (buffer-string)))))
+
 
 \f
 ;;; Editing
-- 
2.35.1


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


-- 
Ihor Radchenko,
Org mode contributor,
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