Re: adaptive-fill-mode and auto-fill-mode

[Stefan Monnier]

>>>> I disagree with the "at left margin" thingy.  I'm not 100% sure what it's
>>>> trying to fix, tho, so please explain which scenario it fixes.
>>> I agree with your disagreement.
>> I'm not sure what you mean: isn't the test still present in your
>> latest patch?

> I removed the "at left margin" thingy.  If you refer to the

> (<= compos (line-beginning-position 0)

> thingy then I need that to avoid the `indent-to-left-margin' in
> `comment-indent-new-line' as in bug(2) of my first mail.  If we agree to
> use `indent-according-to-mode' here, that check is not needed.  Note
> that bug(2) is quite nasty since with longer defuns you usually won't
> guess why it happened in the first place.  And defuns preceded by two
> ";; " comment lines are quite frequent in the Elisp source.

I guess what I don't understand is in what way this is a fix for
the problem.  Yes, it may work for your specific case, but what about in
cases such as:

  (defun foo ()
    "docstring"

    ;; The empty line above is important.
    ;; This second line as well.
    (let ((toto titi))
      ;; some comment klsdhfkshgkslfkhgsjkhgskhgjfdhgd yy

the same problem will happen, although not at BOL.  Maybe a better fix would
be (when compos is set and `prefix' matches
(concat "\\`[ \t]*" comment-start-skip)) to go to compos, and call
comment-forward until we reach point.  If we bump into non-comment before
reaching point, then the prefix was set based on some other unrelated
comment and should be ignored.

>>> BTW is the fill.el change harmless?  In my opinion the
>>> `fill-nobreak-predicate' stuff was broken.  Did anyone ever use that?
>> I think it doesn't matter either way.
> Suppose you write a long Elisp regexp that you don't want to break
> before you finished writing.  Auto fill will break it without mercy.

It's not that it doesn't make a difference, but that it only comes into play
in fairly unusual circumstances and that which behavior is right and which
is wrong depends on the specifics, so you can find supporting evidence
either way.  In this context I won't oppose your patch, especially since it
simplifies the code.

>>> !        ;; Don't accept a prefix in an end-of-line comment that doesn't 
>>> start at
>>> !        ;; line beginning or whose start sequence doesn't match the prefix.
>>> !        ;; This should work around a bug where `do-auto-fill' determines 
>>> the
>>> !        ;; prefix from the beginning of the paragraph but doesn't pay 
>>> attention
>>> !        ;; to comments.
>>> !        (or (not (string-equal comment-end ""))
>>> !      (and (<= compos (line-beginning-position 0))
>>> !           (save-excursion
>>> !             (goto-char compos)
>>> !             (looking-at (regexp-quote prefix))))))
>> 
>> 
>> I don't quite understand this.  I feel like this block repeats the previous
>> one.  I.e. the first part (comment-end check) seems like an inferior
>> alternative to the "comment-forward + not bolp" check above,

> Indeed.  The reason is that I don't want to affect multiline comment
> environments when the comment starting at compos is a single line
> comment.

But in what is checking comment-end better than using "comment-forward + not
bolp"?

> Hence I check whether this is a "single line environment" -
> exemplified by `comment-end' equalling the empty string - and proceed
> only in that case.  If you think that multiline comment modes may be
> affected as well ...

*Many* modes allow both single-line and multi-line comments and
comment-start/comment-end doesn't restrict the comment style that we may
encounter in the file.  I.e. in C++ mode comment-end may be "" but /*..*/
comments are pretty common.

>> ;; foo wrote:
>> ;; > where the fill-prefix will include 3 spaces after ";;"
>> ;; > even though the first line doesn't match it.

> Do you think my patch would be responsible for not handling these

The prefix would be ";; >" which doesn't match the string at compos, so your
code would reject it.

> (at line-beginning)?

These can happen at any indentation.

> If the prefix passed to `comment-valid-prefix-p'
> matches the string at compos there shouldn't be any problems.

Of course, but what I'm saying is that it may not match, even in cases where
it's perfectly correct.

> I think the second example doesn't work because
> `fill-common-string-prefix' determines the common prefix of ";; " and
> ";; > " as ";; " long before I lay my hands at this.

That depends on adaptive-fill-function.

>> I.e. we should probably use `comment-start-skip' on `prefix' to extract the
>> comment-marker part of the prefix, then strip whitespace, then check that
>> the same match&strip at compos returns the same thing.

> I don't quite follow you here.  Where do you want to strip whitespace,
> before the comment marker, between comment marker and body?  Would it be
> reliable to do that?

Why not?

>>> +   (set (make-local-variable 'fill-nobreak-predicate)
>>> +        ;; Try to avoid that auto-fill breaks strings.
>>> +        (lambda ()
>>> +    (and (eq (get-text-property (point) 'face)
>>> +             'font-lock-string-face)
>>> +         (or (= (point) (point-min))
>>> +             (eq (get-text-property (1- (point)) 'face)
>>> +                 'font-lock-string-face)))))
>> 
>> 
>> Do we really want that?  It seems at best to be a personal preference.

> Personally, I don't care at all.  My auto-fill-function uses syntax-ppss
> and doesn't fill normal strings.  But, as stated above, I don't think
> that auto-fill should be allowed to break non-doc-strings in Elisp.

I completely understand your point of view, but it's still far from
a bug-fix and it's not the only way to go about it.  Have you tried
comment-auto-fill-only-comments?


        Stefan