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

[Ihor Radchenko]

Sébastien Miquel  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.

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

>From 8777839a4fe5da6c9a780eac946e1a8a892d4f22 Mon Sep 17 00:00:00 2001
Message-Id: <8777839a4fe5da6c9a780eac946e1a8a892d4f22.1664788728.git.yanta...@gmail.com>
From: Ihor Radchenko 
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 
Fixes: https://orgmode.org/list/201b44de-1f97-1b23-1767-970ee00f2...@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 ( 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


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

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]

Sébastien Miquel  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 ~( 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 .
Support Org development at ,
or support my work at 

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

[Rudolf Adamkovič]

Fabio Natali  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@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č  [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

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

[Fabio Natali]

On 2022-01-10 19:50:59 +, Sébastien Miquel
 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

[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