From: Tim Cross <theophilusx@gmail.com>
To: Kaushal Modi <kaushal.modi@gmail.com>
Cc: Richard Lawrence <richard.lawrence@uni-tuebingen.de>,
Marco Wahl <marcowahlsoft@gmail.com>,
emacs-org list <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] Fix org-comment-line-break-function
Date: Wed, 01 Dec 2021 10:15:07 +1100 [thread overview]
Message-ID: <87zgpli5nc.fsf@gmail.com> (raw)
In-Reply-To: <CAFyQvY2Y+3WzzO0GgFVJ67+=QapzS_Z==3fib4yswzmpSKaFLg@mail.gmail.com>
Kaushal Modi <kaushal.modi@gmail.com> writes:
> On Tue, Nov 30, 2021 at 3:20 PM Marco Wahl <marcowahlsoft@gmail.com> wrote:
>
> 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 am able to reproduce that M-j issue (using Emacs version: GNU Emacs 28.0.60 (build 9, x86_64-pc-linux-gnu, GTK+ Version 3.22.30,
> cairo version 1.15.12)
> of 2021-11-29, built using commit c4daff9cf844ec85930bdcd2064787c92c260861, and Org mode version 9.5
> (release_9.5-292-g5e35de)).
>
> And this patch fixes that for me as well.
>
> +1 for applying this patch.
>
> =====
>
> Before this patch, M-j gave this backtrace with debug enabled:
>
> Debugger entered--Lisp error: (wrong-type-argument char-or-string-p nil)
> insert-before-markers-and-inherit(nil)
> org-comment-line-break-function(nil)
> default-indent-new-line(nil t)
> funcall-interactively(default-indent-new-line nil t)
> call-interactively(default-indent-new-line nil nil)
> command-execute(default-indent-new-line)
>
> Output of C-h k M-j:
>
> M-j runs the command default-indent-new-line (found in global-map),
> which is an interactive compiled Lisp function in ‘simple.el’.
>
> It is bound to C-M-j, M-j.
>
> (default-indent-new-line &optional SOFT FORCE)
>
> Break line at point and indent.
> If a comment syntax is defined, call ‘comment-line-break-function’.
>
> The inserted newline is marked hard if variable ‘use-hard-newlines’ is true,
> unless optional argument SOFT is non-nil.
I'm not sure this is the right patch to apply. While it does fix the
immediate error, it really does so by just avoiding the call to
insert-before-markers-and-inherit when fill-prefix is nil. It does not
address the question of what that function is supposed to do or whether
the correct fix is either to call the function without the fill-prefix
argument (which also works in that it avoids the error) or if instead,
the patch should be
(if fill-prefix
(insert-before-markers-and-inherit fill-prefix)
(insert-before-markers-and-inherit))
I note also that with or without the patch, the function does not appear
to work correctly anyway. If you hit M-j while in a comment, the new
line should be indented appropriately and have the comment character
prefix i.e. start a new comment line. It does not do that. This is
supposed to be the key difference between C-j and M-j.
Regardless, I think that unless we understand the purpose of
insert-before-markers-and-inherit, we should make the patch such that it
still calls that function. Even if fill-prefix is nil there is
probably a good reason why the markers and properties need to be
modified for some situations.
It would be good to get Nicholas' input here as I think he wrote the
original function back in 2012.
next prev parent reply other threads:[~2021-11-30 23:31 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-28 10:26 Is M-j broken for you in Org on Emacs 27 and 28? Richard Lawrence
2021-11-28 14:37 ` Greg Minshall
2021-11-28 14:45 ` Colin Baxter 😺
2021-11-28 20:17 ` Richard Lawrence
2021-11-29 1:18 ` Tim Cross
2021-11-29 8:09 ` Richard Lawrence
2021-11-29 13:35 ` Tim Cross
2021-11-29 15:49 ` Richard Lawrence
2021-11-29 14:31 ` Colin Baxter 😺
2021-11-30 1:10 ` Tim Cross
2021-11-30 17:03 ` [PATCH] Fix org-comment-line-break-function (was: Is M-j broken for you in Org on Emacs 27 and 28?) Richard Lawrence
2021-11-30 20:18 ` [PATCH] Fix org-comment-line-break-function Marco Wahl
2021-11-30 22:06 ` Tim Cross
2021-12-01 8:36 ` Marco Wahl
2021-11-30 22:08 ` Kaushal Modi
2021-11-30 23:15 ` Tim Cross [this message]
2021-11-30 23:53 ` Kaushal Modi
2021-12-01 1:44 ` Kaushal Modi
2021-12-01 6:17 ` Tim Cross
2021-12-01 8:16 ` Richard Lawrence
2021-12-01 13:02 ` Nicolas Goaziou
2021-12-04 22:23 ` Tim Cross
2021-12-05 3:36 ` Kaushal Modi
2021-12-05 9:14 ` Nicolas Goaziou
2021-12-06 13:17 ` Richard Lawrence
2021-12-06 13:51 ` Kaushal Modi
2021-12-11 15:47 ` Nicolas Goaziou
2022-09-29 5:11 ` Ihor Radchenko
[not found] ` <871qro5e4w.fsf@aquinas.mail-host-address-is-not-set>
2022-10-04 11:55 ` Ihor Radchenko
2021-11-30 17:16 ` Is M-j broken for you in Org on Emacs 27 and 28? Morgan Willcock
2021-11-29 8:22 ` Richard Lawrence
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zgpli5nc.fsf@gmail.com \
--to=theophilusx@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=kaushal.modi@gmail.com \
--cc=marcowahlsoft@gmail.com \
--cc=richard.lawrence@uni-tuebingen.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).