[PATCH] org.el: Fix the filling of regions containing lists

[PATCH] org.el: Fix the filling of regions containing lists

[Sébastien Miquel]

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

Hi,

The attached patch fixes the following issues with the functions
=fill-region= and =fill-paragraph=, when called with an active region
containing a list.

In the examples, replace 'long line' by long lines to be filled.

  + Calling =fill-region= on a region which contains a list with single
    line items (such as the one below) breaks the list structure.
    - long line
    - long line
  + Calling =fill-region= on a region with a list such as the one below
    doesn't fill the list
    - short line
      short line
    - short line
      short line
  + Calling =fill-paragraph= on a region containing a list such as the
    one below doesn't fill the first item
    - long line
    - long line
    - long line
  + Calling =fill-paragraph= on a region containing a list such as the
    one below doesn't fill the list
    - long line
    - long line
    - short line

Regards,

-- 
Sébastien Miquel

[-- Attachment #2: 0001-org.el-Fix-the-filling-of-regions-containing-lists.patch --]
[-- Type: text/x-patch, Size: 5615 bytes --]

From 6c60e8e43e39074fa87da2f0ed272a69a6793862 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Sat, 6 Nov 2021 22:41:20 +0100
Subject: [PATCH] org.el: Fix the filling of regions containing lists

* lisp/org.el (org-setup-filling): Set fill-forward-paragraph-function.
(org--single-lines-list-is-paragraph): New internal variable. Whether
a list with single lines items should be considered a single
paragraph.
(org--paragraph-at-point): use org--single-lines-list-is-paragraph.
(org-fill-paragraph): When an active region contains a list ensure
every item get filled.
* testing/lisp/test-org.el (test-org/fill-paragraph):
(test-org/fill-region): Test behaviour of fill-paragraph and
fill-region with an active region containing a list.

When filling paragraphs in a region, do not treat a list with single
lines items as a single paragraph.
---
 lisp/org.el              | 19 +++++++++++++++----
 testing/lisp/test-org.el | 41 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 025513e7a..17046f391 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19629,6 +19629,8 @@ assumed to be significant there."
 ;; `org-setup-filling' installs filling and auto-filling related
 ;; variables during `org-mode' initialization.
 
+(defvar org--single-lines-list-is-paragraph) ; defined later
+
 (defun org-setup-filling ()
   (require 'org-element)
   ;; Prevent auto-fill from inserting unwanted new items.
@@ -19644,6 +19646,10 @@ assumed to be significant there."
     (setq-local paragraph-start paragraph-ending)
     (setq-local paragraph-separate paragraph-ending))
   (setq-local fill-paragraph-function 'org-fill-paragraph)
+  (setq-local fill-forward-paragraph-function
+              (lambda (&optional arg)
+                (let ((org--single-lines-list-is-paragraph nil))
+                  (org-forward-paragraph arg))))
   (setq-local auto-fill-inhibit-regexp nil)
   (setq-local adaptive-fill-function 'org-adaptive-fill-function)
   (setq-local normal-auto-fill-function 'org-auto-fill-function)
@@ -19873,9 +19879,11 @@ filling the current element."
 	    (progn
 	      (goto-char (region-end))
 	      (skip-chars-backward " \t\n")
-	      (while (> (point) start)
-		(org-fill-element justify)
-		(org-backward-paragraph)))
+	      (let ((org--single-lines-list-is-paragraph nil))
+                (while (> (point) start)
+		  (org-fill-element justify)
+		  (org-backward-paragraph)
+                  (skip-chars-backward " \t\n"))))
 	  (goto-char origin)
 	  (set-marker origin nil))))
      (t
@@ -21218,6 +21226,9 @@ It also provides the following special moves for convenience:
     ;; Return moves left.
     arg))
 
+(defvar org--single-lines-list-is-paragraph t
+  "Treat plain lists with an item every line as a whole paragraph")
+
 (defun org--paragraph-at-point ()
   "Return paragraph, or equivalent, element at point.
 
@@ -21279,7 +21290,7 @@ Function may return a real element, or a pseudo-element with type
 	      (while (memq (org-element-type (org-element-property :parent l))
 			   '(item plain-list))
 		(setq l (org-element-property :parent l)))
-	      (and l
+	      (and l org--single-lines-list-is-paragraph
 		   (org-with-point-at (org-element-property :post-affiliated l)
 		     (forward-line (length (org-element-property :structure l)))
 		     (= (point) (org-element-property :contents-end l)))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 056ea7d87..4375456da 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -765,8 +765,49 @@
 	      (push-mark (point) t t)
 	      (goto-char (point-max))
 	      (call-interactively #'org-fill-paragraph)
+	      (buffer-string)))))
+  ;; Fill every list item in a region
+  (should
+   (equal "\n- 2345678\n  9\n- 2345678\n  9"
+	  (org-test-with-temp-text "\n- 2345678 9\n- 2345678 9"
+	    (let ((fill-column 10))
+	      (transient-mark-mode 1)
+	      (push-mark (point-min) t t)
+	      (goto-char (point-max))
+	      (call-interactively #'org-fill-paragraph)
+	      (buffer-string)))))
+  (should
+   (equal "\n- 2345678\n  9\n- 2345678"
+	  (org-test-with-temp-text "\n- 2345678 9\n- 2345678"
+	    (let ((fill-column 10))
+	      (transient-mark-mode 1)
+	      (push-mark (point-min) t t)
+	      (goto-char (point-max))
+	      (call-interactively #'org-fill-paragraph)
 	      (buffer-string))))))
 
+(ert-deftest test-org/fill-region ()
+  "Test `fill-region' behaviour."
+  ;; fill-region should fill every item of a list
+  (should
+   (equal "\n- 2345678\n  9\n- 2345678\n  9"
+	  (org-test-with-temp-text "\n- 2345678 9\n- 2345678 9"
+	                           (let ((fill-column 10))
+	                             (transient-mark-mode 1)
+	                             (push-mark (point-min) t t)
+	                             (goto-char (point-max))
+	                             (call-interactively #'fill-region)
+	                             (buffer-string)))))
+  (should
+   (equal "\n- 1 2\n- 1 2"
+	  (org-test-with-temp-text "\n- 1\n  2\n- 1\n  2"
+	                           (let ((fill-column 10))
+	                             (transient-mark-mode 1)
+	                             (push-mark (point-min) t t)
+	                             (goto-char (point-max))
+	                             (call-interactively #'fill-region)
+	                             (buffer-string)))))  )
+
 (ert-deftest test-org/auto-fill-function ()
   "Test auto-filling features."
   ;; Auto fill paragraph.
-- 
2.34.1

Re: [PATCH] org.el: Fix the filling of regions containing lists

[Samuel Wales]

wow thank you i look forward to this.

On 11/30/21, Sébastien Miquel <sebastien.miquel@posteo.eu> wrote:
> Hi,
>
> The attached patch fixes the following issues with the functions
> =fill-region= and =fill-paragraph=, when called with an active region
> containing a list.
>
> In the examples, replace 'long line' by long lines to be filled.
>
>   + Calling =fill-region= on a region which contains a list with single
>     line items (such as the one below) breaks the list structure.
>     - long line
>     - long line
>   + Calling =fill-region= on a region with a list such as the one below
>     doesn't fill the list
>     - short line
>       short line
>     - short line
>       short line
>   + Calling =fill-paragraph= on a region containing a list such as the
>     one below doesn't fill the first item
>     - long line
>     - long line
>     - long line
>   + Calling =fill-paragraph= on a region containing a list such as the
>     one below doesn't fill the list
>     - long line
>     - long line
>     - short line
>
> Regards,
>
> --
> Sébastien Miquel
>


-- 
The Kafka Pandemic

Please learn what misopathy is.
https://thekafkapandemic.blogspot.com/2013/10/why-some-diseases-are-wronged.html

Re: [PATCH] org.el: Fix the filling of regions containing lists

[Sébastien Miquel]

Applied, as 4e4250061.

Took a little while :P

-- 
Sébastien Miquel