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. From de607dff518eaa91149ff1aa8c255f67fb6ee887 Mon Sep 17 00:00:00 2001 From: Kaushal Modi Date: Tue, 30 Nov 2021 20:37:10 -0500 Subject: [PATCH] org: Remove `org-comment-line-break-function' * lisp/org.el: Remove `org-comment-line-break-function' and let `comment-line-break-function' be the default value. * testing/lisp/test-org.el (test-org/default-indent-new-line): Add tests for M-j behavior when point is inside or outside an Org comment. This fixes the `M-j' binding issue reported by Richard Lawrence in . --- lisp/org.el | 17 ++--------------- testing/lisp/test-org.el | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index ec59ddf44..ee8ca1f03 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -19624,8 +19624,7 @@ assumed to be significant there." ;; `org-auto-fill-function' takes care of auto-filling. It calls ;; `do-auto-fill' only on valid areas with `fill-prefix' shadowed with -;; `org-adaptive-fill-function' value. Internally, -;; `org-comment-line-break-function' breaks the line. +;; `org-adaptive-fill-function' value. ;; `org-setup-filling' installs filling and auto-filling related ;; variables during `org-mode' initialization. @@ -19647,8 +19646,7 @@ assumed to be significant there." (setq-local fill-paragraph-function 'org-fill-paragraph) (setq-local auto-fill-inhibit-regexp nil) (setq-local adaptive-fill-function 'org-adaptive-fill-function) - (setq-local normal-auto-fill-function 'org-auto-fill-function) - (setq-local comment-line-break-function 'org-comment-line-break-function)) + (setq-local normal-auto-fill-function 'org-auto-fill-function)) (defun org-fill-line-break-nobreak-p () "Non-nil when a new line at point would create an Org line break." @@ -19903,17 +19901,6 @@ filling the current element." (adaptive-fill-mode (not (equal fill-prefix "")))) (when fill-prefix (do-auto-fill)))))) -(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)) - ;;; Fixed Width Areas diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el index 056ea7d87..de4ac7ea9 100644 --- a/testing/lisp/test-org.el +++ b/testing/lisp/test-org.el @@ -1200,6 +1200,25 @@ (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" + (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" + (call-interactively #'default-indent-new-line) + (buffer-string)))) + (should + (equal "# Some Org\n# comment" + (org-test-with-temp-text "# Some Org comment" + (call-interactively #'default-indent-new-line) + (buffer-string))))) ;;; Editing -- 2.34.0 -- Kaushal Modi On Mon, Dec 6, 2021 at 8:17 AM Richard Lawrence < richard.lawrence@uni-tuebingen.de> wrote: > Kaushal Modi writes: > > > On Sat, Dec 4, 2021, 5:25 PM Tim Cross wrote: > > > >> Given that Nicholas cannot remember the reason for the original function > >> and suspects it was meant to be an internal only function, I think this > >> patch is probably the best way forward and should be applied. > > > > Thanks. Nicolas asked me to add tests for this patch. But I need to look > > into how to add tests for behavior of bindings. Need to add tests for M-j > > binding behavior when cursor is within a comment or outside. > > FWIW I have been running the equivalent of Kaushal's patch (I just > removed the local binding of comment-line-break-function to > 'org-comment-line-break-function without deleting the function) and have > also set > > (debug-on-entry 'org-comment-line-break-function) > > I've been running this for at least a week with no issues, and have > never been dropped into the debugger. So at least the testing through my > normal work indicates there are no problems with the patch. > > -- > Best, > Richard >