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

From: [email-masked]
Date: Wed, 23 Aug 2017 22:16:59 -0400

Samuel Freilich <[email-masked]> writes:

> tab-width = 4 seems consistent with how the rest of the file is formatted.
> I didn't want to change the spacing in lines not touched by my diff.

Hmm, somehow the code that landed in my buffer looked wrongly indented.
Something got messed up somewhere (possibly by the email system,
possibly during patch application, I've been experimenting with various
forms of automation for that), but since the indentation in your next
patch looks fine there's not really much point in worrying about it.

> I added tests and a change description, and formatted the patch according
> to the current instructions.

Thanks!  

> I made the patch a bit more minimal. I pruned the part that dealt with
> adaptive-fill-first-line-regexp didn't work as well as expected, since
> that didn't work as well as expected (it didn't deal with all the
> complexity possible with adaptive-fill-function). The updated version
> at least handles cases where the fill-prefix isn't shorter than the
> first-line prefix. That allowed me to simplify the code quite a bit,
> since that makes the previous logic for skipping the exact fill-prefix
> redundant, fill-move-to-break-point already handles the logic of
> trying to skip at least one word after the start position passed to
> it. Please take another look.

Your commit message is a bit too focused on the problems of what was
there previously rather than how your new code is correct.  This
description in your email is better, but I'm still struggling a bit (I'm
realizing I was probably a bit too superficial previously, there are a
lot of interacting options which makes things tricky).

If I understand correctly, the previous code would take the fill-prefix
as non-breakable if the current line started with the fill-prefix.  Your
new code takes the first N characters of the line as non-breakable
(where N is the length of fill-prefix, should this be based on
string-width instead?).  Is that correct (for both adaptive-fill-mode
and otherwise)?  If so, please add an explanation of why it's correct to
the commit message.  If not, I hope we can elaborate the commit message
until even I can understand what's happening ;)

(Minor commit message format nitpick: the part explaining the changes to
the function should start with "* lisp/simple.el (do-auto-fill):".)