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