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

[Tim Cross <theophilusx@gmail.com>]

Marco Wahl <marcowahlsoft@gmail.com> writes:

> Hi Richard and all,
>
> [...]
>
>> Just to be extra, super sure, I built Emacs this afternoon from a
>> checkout of the repo, and the error is *still* there, with the same
>> cause. In that build, with emacs -Q, I have:
>>
>> (org-version)
>> "9.5"
>>
>> (emacs-version)
>> "GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0)
>>  of 2021-11-30"
>>
>> At this point I've replicated the bug on my machine in four different
>> builds of Emacs (version 26.1 from Debian, 27.2 and "emacs-next" from
>> Guix, and version 29.0.50 I built myself from source) with several
>> versions of Org (the built-in ones in these Emacsen and a recent build
>> of the bugfix branch). It is robustly reproducible for me, and the cause
>> is clear: default-indent-new-line calls org-comment-line-break-function,
>> which calls
>>
>> (insert-before-markers-and-inherit nil)
>>
>> which is a type error. I'm looking for help figuring out what the right
>> fix is. I attach a patch for the simplest fix I can think of; please let
>> me know if something else would be better.
>
>     diff --git a/lisp/org.el b/lisp/org.el
>     index 1a1375461..fdeec0d67 100644
>     --- a/lisp/org.el
>     +++ b/lisp/org.el
>     @@ -19695,7 +19695,8 @@ non-nil."
>        (save-excursion (forward-char -1) (delete-horizontal-space))
>        (delete-horizontal-space)
>        (indent-to-left-margin)
>     -  (insert-before-markers-and-inherit fill-prefix))
>     +  (when fill-prefix
>     +    (insert-before-markers-and-inherit fill-prefix)))
>  
> I don't have anything better.  I think this is a good patch.  It makes
> M-j work again.
>
> Possible refinements and improvements can follow.
>
> +1 for applying of your patch.
>
>

I was finally able to reproduce the error. It depends to some degree on
the text in the buffer and where the cursor is when you hit M-j. Adding
some additional text and moving the cursor to different locations
enabled me to reproduce the error and I now agree it is a bug in
org-comment-line-break-function.

I don't know if your patch is the right fix or not because I don't
understand what the purpose of insert-before-marks-and-inherit is - in
fact, the doc string for that function doesn't even state what the @rest
args argument is for, so I don't understand why it is passing in
fill-prefix. For example, is it safe to assume
insert-before-merks-and-inherit does not need to be called if
fill-prefix is nil? Why is that function even called with the
fill-prefix as an argument?