bug#20774: auto-fill doesn't work properly when first-line prefix differs in adaptive-fill-mode

[npostavs@users.sourceforge.net]

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

[forwarding to bug list]


[-- Attachment #2: Type: message/rfc822, Size: 5302 bytes --]

[-- Attachment #2.1.1.1: Type: text/plain, Size: 517 bytes --]

<npostavs@users.sourceforge.net> wrote:

> Actually, it's now occuring to me that adding a position to a
> string-width isn't quite correct either

Ah right, (point) is in characters, which doesn't correspond directly to
columns at all. I think I can avoid the extra save-excursion and make it a
little cleaner as a result.

> there should be 2 spaces after the period

Heresy! :-P

But done. That does seem to be the most common style in the ChangeLog.

Please take another look. Hopefully this patch can be merged.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1.1.2: 0003-Do-not-split-line-before-width-of-fill-prefix.patch --]
[-- Type: text/x-patch; name=0003-Do-not-split-line-before-width-of-fill-prefix.patch, Size: 3440 bytes --]

From 118c8a43510a7d3020a552ce0e87e5a1b9ccec54 Mon Sep 17 00:00:00 2001
From: Samuel Freilich <sfreilich@google.com>
Date: Wed, 23 Aug 2017 13:40:45 -0400
Subject: [PATCH] Do not split line before width of fill-prefix

When auto-filling a paragraph, don't split a line before the width of the
fill-prefix, creating a subsequent line that is as long or longer (Bug#20774).
* lisp/simple.el (do-auto-fill): Only consider break-points that are later in
the line than the width of the fill-prefix.  This is a more general solution
than the previous logic, which only skipped over the exact fill-prefix.  The
fill-prefix doesn't necessarily match the prefix of the first line of a
paragraph in adaptive-fill-mode.
---
 lisp/simple.el            | 27 ++++++++++++---------------
 test/lisp/simple-tests.el | 11 +++++++++++
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 13cfa3487d..27990bb661 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -7151,18 +7151,18 @@ do-auto-fill
 	       (setq fill-prefix prefix))))
 
       (while (and (not give-up) (> (current-column) fc))
-	;; Determine where to split the line.
-	(let* (after-prefix
-	       (fill-point
-		(save-excursion
-		  (beginning-of-line)
-		  (setq after-prefix (point))
-		  (and fill-prefix
-		       (looking-at (regexp-quote fill-prefix))
-		       (setq after-prefix (match-end 0)))
-		  (move-to-column (1+ fc))
-		  (fill-move-to-break-point after-prefix)
-		  (point))))
+        ;; Determine where to split the line.
+        (let ((fill-point
+               (save-excursion
+                 (beginning-of-line)
+                 ;; Don't split earlier in the line than the length of the
+                 ;; fill prefix, since the resulting line would be longer.
+                 (when fill-prefix
+                   (move-to-column (string-width fill-prefix)))
+                 (let ((after-prefix (point)))
+                    (move-to-column (1+ fc))
+                    (fill-move-to-break-point after-prefix)
+                    (point)))))
 
 	  ;; See whether the place we found is any good.
 	  (if (save-excursion
@@ -7170,9 +7170,6 @@ do-auto-fill
 		(or (bolp)
 		    ;; There is no use breaking at end of line.
 		    (save-excursion (skip-chars-forward " ") (eolp))
-		    ;; It is futile to split at the end of the prefix
-		    ;; since we would just insert the prefix again.
-		    (and after-prefix (<= (point) after-prefix))
 		    ;; Don't split right after a comment starter
 		    ;; since we would just make another comment starter.
 		    (and comment-start-skip
diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index ad7aee1db1..c7330e4034 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -497,5 +497,16 @@ simple-test-undo-with-switched-buffer
       (should (equal (line-number-at-pos 5) 3))
       (should (equal (line-number-at-pos 7) 4)))))
 
+(ert-deftest auto-fill-mode-no-break-before-length-of-fill-prefix ()
+  (with-temp-buffer
+    (setq-local fill-prefix "   ")
+    (set-fill-column 5)
+    ;; Shouldn't break after 'foo' (3 characters) when the next
+    ;; line is indented >= to that, that woudln't result in shorter
+    ;; lines.
+    (insert "foo bar")
+    (do-auto-fill)
+    (should (string-equal (buffer-string) "foo bar"))))
+
 (provide 'simple-test)
 ;;; simple-test.el ends here
-- 
2.14.1.342.g6490525c54-goog