`org-fill-paragraph' (`M-q') in Org Mode source blocks

`org-fill-paragraph' (`M-q') in Org Mode source blocks

[Fabio Natali]

Hello,

I'm having some issues with `M-q' (`org-fill-paragraph') within a Org
Mode source block.

Consider, for instance, a Org Mode file that contains the following
source block.

┌────
│ #+BEGIN_SRC elisp
│ ;; A comment
│ (+ 2 2)
│ #+END_SRC
└────

What happens: when calling `M-q' from within the block, the content is
handled like generic text and transformed as follows.

┌────
│ #+BEGIN_SRC elisp
│   ;; A comment (+ 2 2)
│ #+END_SRC
└────

What I'd be ideally expecting: the code to be potentially transformed
the same way it'd be in Emacs Lisp major mode.

What I'm actually expecting: the above might be too much of a high
expectation (i.e. for Org Mode to be aware of different `fill-paragraph'
mechanisms for different languages). As a second best, I'd be expecting
Org Mode to simply ignore my `M-q' command.

Here are some further considerations:

• Things work as expected when editing the block with the relevant major
  mode, e.g. Emacs Lisp; I know that this can be easily activated with
  `org-edit-special' (`C-c '').
• This can be replicated over different languages, i.e. it doesn't seem
  to be related to Emacs Lisp code in any way.
• I was able to reproduce this with a minimal `init.el' that only
  contains `(org-babel-do-load-languages 'org-babel-load-languages
  '((emacs-lisp . t)))'.
• I've been testing this with GNU Emacs 27.2 and Org Mode 9.5.1.

Also, I can see `M-q' is bound to `org-fill-paragraph'. The [source
code] for that function says:

      This function only applies to comment blocks, comments,
      example blocks and paragraphs.

This would seem to confirm my expectation, i.e. that the command
shouldn't be doing anything within a source block. Instead,
`org-fill-paragraph' seems to be calling `org-fill-element' and then
ultimately `fill-paragraph', [here].

This might be a relevant section:

┌────
│ (cl-case (org-element-type element)
│   ;; Use major mode filling function is source blocks.
│   (src-block (org-babel-do-in-edit-buffer
│ 	      (push-mark (point-min))
│ 	      (goto-char (point-max))
│ 	      (setq mark-active t)
│ 	      (funcall-interactively #'fill-paragraph justify 'region)))
└────

In order to honour its promise of only applying "to comment blocks,
comments, example blocks and paragraphs", shouldn't rather the function
do nothing in this case?

Is there anything obvious that you think I'm missing here? Is anyone
able to replicate the above behaviour and, if so, do you also find it
slightly problematic? Any thoughts and feedback on the above will be
greatly appreciated. :)

Thanks, best, F.


[source code]
<https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/org.el#n19850>

[here]
<https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/org.el#n19757>

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Sébastien Miquel]

Hi,

Fabio Natali writes:
> Is there anything obvious that you think I'm missing here? Is anyone
> able to replicate the above behaviour and, if so, do you also find it
> slightly problematic? Any thoughts and feedback on the above will be
> greatly appreciated.:)

It's not just you. I think org-fill-paragraph should try to act
natively if called from inside a src block.

As it happens, I've recently added the following advice to my init
file.

(defun my-org-fill-paragraph-natively-maybe ()
   (let* ((element (save-excursion (beginning-of-line) 
(org-element-at-point-no-context)))
          (type (org-element-type element)))
     (if (and (eq type 'src-block)
              (> (line-beginning-position)
                 (org-element-property :post-affiliated element))
              (< (line-beginning-position)
                 (org-with-point-at (org-element-property :end element)
                   (skip-chars-backward " \t\n")
                   (line-beginning-position))))
         (progn
           (org-babel-do-in-edit-buffer (fill-paragraph))
           nil) t)))
(advice-add 'org-fill-paragraph :before-while 
#'my-org-fill-paragraph-natively-maybe)

Regards,

-- 
Sébastien Miquel

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Fabio Natali]

On 2022-01-10 19:50:59 +0000, Sébastien Miquel
<sebastien.miquel@posteo.eu> wrote:
[...]
> It's not just you. I think org-fill-paragraph should try to act
> natively if called from inside a src block.

Hi Sébastien,

Thanks for getting back to me and thank you very much for the code
snippet, which I think I'm going to integrate in my configuration.

I'd be curious to hear from other fellow Org Moders. If this is a
relatively common problem and there's interest around it, maybe we could
think of a fix directly in the code base.

Thanks, best wishes, F.

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Rudolf Adamkovič]

Fabio Natali <me@fabionatali.com> writes:

> I'd be curious to hear from other fellow Org Moders.

Thank you!  I wanted to report this exact bug sometime soon.  What I
would like to do (every day):

1. select the entire buffer with "C-x h" and
2. fill all content with "C-q".

As of now, Org has issues with source blocks, plain lists, and more.  I
reported the bug with plain lists here:

https://list.orgmode.org/87ee5z6wa7.fsf@kyleam.com/

In practice, when I ask Org to fill a complicated buffer, it almost
always screws up its content, and often without the ability to undo, or
it freezes the entire Emacs in some infinite loop or something.

Rudy
-- 
"'Contrariwise,' continued Tweedledee, 'if it was so, it might be; and
if it were so, it would be; but as it isn't, it ain't. That's logic.'"
-- Lewis Carroll, Through the Looking Glass

Rudolf Adamkovič <salutis@me.com> [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Sébastien Miquel]

Fabio Natali writes:
> Thanks for getting back to me and thank you very much for the code
> snippet, which I think I'm going to integrate in my configuration.

Thank you for the report. With regard to the snippet, It seems the
advice function needs ~(&optional justify region)~ as arguments rather
than ().

> I'd be curious to hear from other fellow Org Moders. If this is a
> relatively common problem and there's interest around it, maybe we could
> think of a fix directly in the code base.
The best way to get it done is to post a patch to the list. If it
doesn't break a legitimate existing behaviour, It should get applied.
I'll probably do so eventually, or you could, if you feel so inclined.
Perhaps one difficulty is to deal with the case where the org-babel
call errors out, say if there's no mode to edit the src code in.

-- 
Sébastien Miquel

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Ihor Radchenko]

Sébastien Miquel <sebastien.miquel@posteo.eu> writes:

> Fabio Natali writes:
>> Thanks for getting back to me and thank you very much for the code
>> snippet, which I think I'm going to integrate in my configuration.
>
> Thank you for the report. With regard to the snippet, It seems the
> advice function needs ~(&optional justify region)~ as arguments rather
> than ().

I am still getting the described behaviour.
However, it does not happen in Org mode itself.
`fill-paragraph' in emacs-lisp-mode does exactly the observed behaviour.

Try
1. emacs -Q
2. insert
;; A comment
(+ 2 2)
3. M-h M-q

Is it emacs-lisp-mode bug? Or is it illegal to fill-region in source
code buffers?

-- 
Ihor Radchenko // yantar92,
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>

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Sébastien Miquel]

Hi,

Ihor Radchenko writes:
> I am still getting the described behaviour.
> However, it does not happen in Org mode itself.
> `fill-paragraph' in emacs-lisp-mode does exactly the observed behaviour.
> 
> Try
> 1. emacs -Q
> 2. insert
> ;; A comment
> (+ 2 2)
> 3. M-h M-q
> 
> Is it emacs-lisp-mode bug? Or is it illegal to fill-region in source
> code buffers?

I think the original report is about M-q, not M-h M-q. M-q behaves as 
expected in emacs-lisp-mode.

Currently, org-fill-paragraph will not work properly when called inside 
src blocks. This can be easily fixed, but probably requires yet another 
org-fill-paragraph-act-natively variable.

-- 
Sébastien Miquel

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Ihor Radchenko]

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

Sébastien Miquel <sebastien.miquel@posteo.eu> writes:

>> Try
>> 1. emacs -Q
>> 2. insert
>> ;; A comment
>> (+ 2 2)
>> 3. M-h M-q
>> 
>> Is it emacs-lisp-mode bug? Or is it illegal to fill-region in source
>> code buffers?
>
> I think the original report is about M-q, not M-h M-q. M-q behaves as 
> expected in emacs-lisp-mode.
>
> Currently, org-fill-paragraph will not work properly when called inside 
> src blocks. This can be easily fixed, but probably requires yet another 
> org-fill-paragraph-act-natively variable.

See the attached.
I am not sure if we really need the variable.
AFAIU, acting natively was the default in the past for M-q with no
region selection. Then, I "fixed" some bug report in 05ee1e6ee06db and
introduced the bug herein.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-fill-element-Respect-region-selection-when-filli.patch --]
[-- Type: text/x-patch, Size: 2130 bytes --]

From 8777839a4fe5da6c9a780eac946e1a8a892d4f22 Mon Sep 17 00:00:00 2001
Message-Id: <8777839a4fe5da6c9a780eac946e1a8a892d4f22.1664788728.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Mon, 3 Oct 2022 17:03:15 +0800
Subject: [PATCH] org-fill-element: Respect region selection when filling
 src-block

* lisp/org.el (org-fill-element): When region is not active, run
`fill-paragraph' at point inside src block.  When region is active and
within src block boundaries, run `fill-paragraph' preserving the
region.  When region is active and crosses src block boundaries, fill
the whole src block.

Reported-by: Fabio Natali <me@fabionatali.com>
Fixes: https://orgmode.org/list/201b44de-1f97-1b23-1767-970ee00f259c@posteo.eu
---
 lisp/org.el | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 036384a04..23cf4198e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19135,11 +19135,18 @@ (defun org-fill-element (&optional justify)
       ;; the buffer.  In that case, ignore filling.
       (cl-case (org-element-type element)
 	;; Use major mode filling function is source blocks.
-	(src-block (org-babel-do-in-edit-buffer
-                    (push-mark (point-min))
-                    (goto-char (point-max))
-                    (setq mark-active t)
-                    (funcall-interactively #'fill-paragraph justify 'region)))
+        (src-block
+         (let ((regionp (region-active-p)))
+           (org-babel-do-in-edit-buffer
+            ;; `org-babel-do-in-edit-buffer' will preserve region if it
+            ;; is within src block contents.  Otherwise, the region
+            ;; crosses src block boundaries.  We re-fill the whole src
+            ;; block in such scenario.
+            (when (and regionp (not (region-active-p)))
+              (push-mark (point-min))
+              (goto-char (point-max))
+              (setq mark-active t))
+            (funcall-interactively #'fill-paragraph justify 'region))))
 	;; Align Org tables, leave table.el tables as-is.
 	(table-row (org-table-align) t)
 	(table
-- 
2.35.1


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


-- 
Ihor Radchenko // yantar92,
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>

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Sébastien Miquel]

Ihor Radchenko writes:
> See the attached.
> I am not sure if we really need the variable.
> AFAIU, acting natively was the default in the past for M-q with no
> region selection. Then, I "fixed" some bug report in 05ee1e6ee06db and
> introduced the bug herein.

You're right, I had not realised org-fill-element already acted natively.

Your patch looks and tests good to me.

-- 
Sébastien Miquel

Re: `org-fill-paragraph' (`M-q') in Org Mode source blocks

[Ihor Radchenko]

Sébastien Miquel <sebastien.miquel@posteo.eu> writes:

> Ihor Radchenko writes:
>> See the attached.
>> I am not sure if we really need the variable.
>> AFAIU, acting natively was the default in the past for M-q with no
>> region selection. Then, I "fixed" some bug report in 05ee1e6ee06db and
>> introduced the bug herein.
>
> You're right, I had not realised org-fill-element already acted natively.
>
> Your patch looks and tests good to me.

Thanks for checking.
Applied onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2c8bd0cc9b914b4dcc11e337faedbabe195097fb

-- 
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

Fixed.