unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: npostavs@users.sourceforge.net
To: Samuel Freilich <sfreilich@google.com>
Cc: 20774@debbugs.gnu.org
Subject: bug#20774: auto-fill doesn't work properly when first-line prefix differs in adaptive-fill-mode
Date: Wed, 23 Aug 2017 22:16:59 -0400	[thread overview]
Message-ID: <87r2w1aew4.fsf@users.sourceforge.net> (raw)
In-Reply-To: <CACQEUgPGbtCQJtNc7U+Zp_0BFPUqCzTfAAMQURHk2+Okmh2wEA@mail.gmail.com> (Samuel Freilich's message of "Wed, 23 Aug 2017 14:20:20 -0400")

Samuel Freilich <sfreilich@google.com> 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):".)





  reply	other threads:[~2017-08-24  2:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 23:39 bug#20774: auto-fill doesn't work properly when first-line prefix differs in adaptive-fill-mode Samuel Freilich
2015-06-26 23:40 ` bug#20774: Typo in Patch Samuel Freilich
2017-08-22 12:49 ` bug#20774: auto-fill doesn't work properly when first-line prefix differs in adaptive-fill-mode npostavs
2017-08-22 16:00   ` Samuel Freilich
2017-08-23  3:56     ` npostavs
2017-08-23 18:20       ` Samuel Freilich
2017-08-24  2:16         ` npostavs [this message]
2017-08-24 15:11           ` Samuel Freilich
2017-08-25  1:45             ` npostavs
2017-08-29  3:37 ` npostavs
2017-08-29  3:55   ` npostavs
2017-08-31  0:51     ` npostavs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r2w1aew4.fsf@users.sourceforge.net \
    --to=npostavs@users.sourceforge.net \
    --cc=20774@debbugs.gnu.org \
    --cc=sfreilich@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).