bug#72870: [PATCH] Flow fill texts after the last hard newline

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Pengji Zhang]

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

Tags: patch  Hello!

I found that function `fill-flowed-encode' would not fill texts 
after the last hard newline in a buffer. To reproduce, run 'emacs 
-Q' and then evaluate the following snippet:

--8<---------------cut here---------------start------------->8---
(with-current-buffer (get-buffer-create "*tmp-f=f*")
  (use-hard-newlines)
  (insert "1\n2")
  (fill-flowed-encode)
  (display-buffer (current-buffer)))
--8<---------------cut here---------------end--------------->8---

The expected output should be '1 2', but the actual is '1\n2'.

The first patch changes this.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Flow-fill-texts-after-the-last-hard-newline.patch --]
[-- Type: text/patch, Size: 1218 bytes --]

From 9320c59a212459181f81025e9895e9172dde1361 Mon Sep 17 00:00:00 2001
From: Pengji Zhang <me@pengjiz.com>
Date: Wed, 28 Aug 2024 18:57:35 +0800
Subject: [PATCH 1/2] Flow fill texts after the last hard newline

* lisp/mail/flow-fill.el (fill-flowed-encode): Use `(point-max)'
as `end' if there are no remaining hard newlines in buffer.
This ensures that the last paragraph will always be encoded,
even without a trailing hard newline.
---
 lisp/mail/flow-fill.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/mail/flow-fill.el b/lisp/mail/flow-fill.el
index 919490ec5aa..d4ad7d45982 100644
--- a/lisp/mail/flow-fill.el
+++ b/lisp/mail/flow-fill.el
@@ -78,7 +78,9 @@ fill-flowed-encode
       (let ((start (point-min)) end)
 	;; Go through each paragraph, filling it and adding SPC
 	;; as the last character on each line.
-	(while (setq end (text-property-any start (point-max) 'hard 't))
+	(while (and (< start (point-max))
+                    (setq end (or (text-property-any start (point-max) 'hard 't)
+                                  (point-max))))
 	  (save-restriction
 	    (narrow-to-region start end)
 	    (let ((fill-column (eval fill-flowed-encode-column t)))
-- 
2.46.0


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


Besides, I found that the test case 
'fill-flow-tests-fill-flowed-encode' in 'fill-flow-tests' does not 
actually test the function 'fill-flowed-encode', which requires 
'use-hard-newlines'[1], and the test input does not cover some 
behaviors of the function. The second patch improves it a bit.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Improve-tests-for-flow-fill.patch --]
[-- Type: text/x-patch, Size: 3799 bytes --]

From 649509a3ce4fbc773ed9490b1dd214f1c18dea1e Mon Sep 17 00:00:00 2001
From: Pengji Zhang <me@pengjiz.com>
Date: Wed, 28 Aug 2024 19:11:35 +0800
Subject: [PATCH 2/2] Improve tests for flow-fill

* test/lisp/mail/flow-fill-tests.el
(fill-flow-tests-fill-flowed-decode): Remove debug message.
(fill-flow-tests-fill-flowed-encode): Actually test function
`fill-flowed-encode', which requires `use-hard-newline' to be
non-nil.  Also adjust test input to check behaviors of the
function under different cases.
---
 test/lisp/mail/flow-fill-tests.el | 42 ++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/test/lisp/mail/flow-fill-tests.el b/test/lisp/mail/flow-fill-tests.el
index 1f698f538f3..50fbb1e7f80 100644
--- a/test/lisp/mail/flow-fill-tests.el
+++ b/test/lisp/mail/flow-fill-tests.el
@@ -54,37 +54,43 @@ fill-flow-tests-fill-flowed-decode
     (with-temp-buffer
       (insert input)
       (fill-flowed)
-      (message "foo")
       (should (equal (buffer-string) output)))))
 
 (ert-deftest fill-flow-tests-fill-flowed-encode ()
   (let ((input
          (concat
-          "> Thou villainous ill-breeding spongy dizzy-eyed \n"
-          "> reeky elf-skinned pigeon-egg! \n"
-          ">> Thou artless swag-bellied milk-livered \n"
-          ">> dismal-dreaming idle-headed scut!\n"
+          ;; Hard newline in the middle of a level
+          "> Thou villainous ill-breeding spongy dizzy-eyed" hard-newline
+          "> reeky elf-skinned pigeon-egg!\n"
+          ">> Thou artless swag-bellied milk-livered\n"
+          ;; Hard new line at the end of a level
+          ">> dismal-dreaming idle-headed scut!" hard-newline
+          ;; Trailing space should be preserved after filling
           ">>> Thou errant folly-fallen spleeny reeling-ripe \n"
           ">>> unmuzzled ratsbane!\n"
-          ">>>> Henceforth, the coding style is to be strictly \n"
+          ">>>> Henceforth, the coding style is to be strictly\n"
           ">>>> enforced, including the use of only upper case.\n"
-          ">>>>> I've noticed a lack of adherence to the coding \n"
+          ;; Consecutive hard newlines within a level
+          ">>>>> I've noticed a lack of adherence to" hard-newline
+          ">>>>> the coding" hard-newline
           ">>>>> styles, of late.\n"
           ">>>>>> Any complaints?\n"))
         (output
          (concat
-          "> Thou villainous ill-breeding spongy dizzy-eyed \n"
+          "> Thou villainous ill-breeding spongy dizzy-eyed\n"
           "> reeky elf-skinned pigeon-egg! \n"
-          ">> Thou artless swag-bellied milk-livered \n"
-          ">> dismal-dreaming idle-headed scut!\n"
-          ">>> Thou errant folly-fallen spleeny reeling-ripe \n"
-          ">>> unmuzzled ratsbane!\n"
-          ">>>> Henceforth, the coding style is to be strictly \n"
-          ">>>> enforced, including the use of only upper case.\n"
-          ">>>>> I've noticed a lack of adherence to the coding \n"
-          ">>>>> styles, of late.\n"
-          ">>>>>> Any complaints?\n"))
-        (fill-flowed-display-column 69))
+          ">> Thou artless swag-bellied milk-livered dismal-dreaming \n"
+          ">> idle-headed scut!\n"
+          ">>> Thou errant folly-fallen spleeny reeling-ripe  unmuzzled \n"
+          ">>> ratsbane! \n"
+          ">>>> Henceforth, the coding style is to be strictly enforced, \n"
+          ">>>> including the use of only upper case. \n"
+          ">>>>> I've noticed a lack of adherence to\n"
+          ">>>>> the coding\n"
+          ">>>>> styles, of late. \n"
+          ">>>>>> Any complaints? \n"))
+        (use-hard-newlines t)
+        (fill-flowed-encode-column 66))
     (with-temp-buffer
       (insert input)
       (fill-flowed-encode)
-- 
2.46.0


[-- Attachment #5: Type: text/plain, Size: 428 bytes --]


The second patch depends on the first one, without which the new 
test will fail. So I decided to submit both together. Please let 
me know if it is better to file another report.

Thanks!

Cheers,
Pengji

PS I have sent an email to assign@gnu.org, but have not got a 
response yet. I think I have not finished the copyright assignment 
process?

[1] https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/mail/flow-fill.el#n77

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Eli Zaretskii]

> From: Pengji Zhang <me@pengjiz.com>
> Date: Thu, 29 Aug 2024 18:44:36 +0800
> 
> I found that function `fill-flowed-encode' would not fill texts 
> after the last hard newline in a buffer. To reproduce, run 'emacs 
> -Q' and then evaluate the following snippet:
> 
> --8<---------------cut here---------------start------------->8---
> (with-current-buffer (get-buffer-create "*tmp-f=f*")
>   (use-hard-newlines)
>   (insert "1\n2")
>   (fill-flowed-encode)
>   (display-buffer (current-buffer)))
> --8<---------------cut here---------------end--------------->8---
> 
> The expected output should be '1 2', but the actual is '1\n2'.
> 
> The first patch changes this.
> 
> Besides, I found that the test case 
> 'fill-flow-tests-fill-flowed-encode' in 'fill-flow-tests' does not 
> actually test the function 'fill-flowed-encode', which requires 
> 'use-hard-newlines'[1], and the test input does not cover some 
> behaviors of the function. The second patch improves it a bit.
> 
> The second patch depends on the first one, without which the new 
> test will fail. So I decided to submit both together. Please let 
> me know if it is better to file another report.

Thanks.

AFAICT, this function exists for the special case of decoding email
messages.  Can email messages have flowed-encoded text without a hard
newline at the end?

> PS I have sent an email to assign@gnu.org, but have not got a 
> response yet. I think I have not finished the copyright assignment 
> process?

No.  Please ping them and CC me, so we could get your paperwork
process going.

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Pengji Zhang]

Eli Zaretskii <eliz@gnu.org> writes:

> AFAICT, this function exists for the special case of decoding 
> email messages. Can email messages have flowed-encoded text 
> without a hard newline at the end? 

I think you meant "encoding"? Anyway, if I understand it 
correctly, an email message should always end with a newline, but 
it is not necessarily a hard newline. For example, 'message.el' 
inserts a normal newline:

  https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/gnus/message.el#n4944

I am not sure what other MUAs do, though.
 
>> PS I have sent an email to assign@gnu.org, but have not got a 
>> response yet. I think I have not finished the copyright 
>> assignment  process? 
> 
> No.  Please ping them and CC me, so we could get your paperwork 
> process going. 

Will do. Thanks!

Pengji

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Pengji Zhang]

Pengji Zhang <me@pengjiz.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes: 
> 
>> AFAICT, this function exists for the special case of decoding 
>> email messages. Can email messages have flowed-encoded text 
>> without a hard newline at the end?  
> 
> I think you meant "encoding"? Anyway, if I understand it 
> correctly, an email message should always end with a newline, 
> but  it is not necessarily a hard newline. For example, 
> 'message.el'  inserts a normal newline: 
> 
>   https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/gnus/message.el#n4944 

I just realized this insertion of newline happens after encoding, 
so this is irrelevant here. Sorry.

It seems that during encoding, no such a newline is added at all. 
To reproduce:

  - Run 'emacs -Q'
  - C-x m C-c C-b
  - M-x use-hard-newlines RET n
  - C-x f 10 RET
  - Eval: (dotimes (i 10) (insert "word "))
  - RET RET
  - Eval: (dotimes (i 10) (insert "word "))
  - M-q
  - C-u M-x mml-preview RET

In the preview buffer, the first paragraph is filled but the 
second is not.

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Pengji Zhang]

Eli Zaretskii <eliz@gnu.org> writes:

>> PS I have sent an email to assign@gnu.org, but have not got a
>> response yet. I think I have not finished the copyright assignment
>> process?
>
> No. Please ping them and CC me, so we could get your paperwork process
> going.

Hi Eli! Yesterday I was told that my assignment process had been
complete.

bug#72870: [PATCH] Flow fill texts after the last hard newline

[Eli Zaretskii]

> From: Pengji Zhang <me@pengjiz.com>
> Cc: 72870@debbugs.gnu.org
> Date: Wed, 09 Oct 2024 17:02:32 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> PS I have sent an email to assign@gnu.org, but have not got a
> >> response yet. I think I have not finished the copyright assignment
> >> process?
> >
> > No. Please ping them and CC me, so we could get your paperwork process
> > going.
> 
> Hi Eli! Yesterday I was told that my assignment process had been
> complete.

Thanks, so I've now installed the patches, and I'm closing this bug.