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

[martin rudalics <rudalics@gmx.at>]

 >>>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.

 >>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.

 >>!        ;; 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.  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 ...

 > and the second
 > part seems to be a stricter form of the second part above (it checks not
 > just that prefix matches comment-start-skip but that it specifically
 > matches the prefix used in the current comment).
 >
 > Note that the stricter match is sometimes too strict.  I often have comments
 > where the adaptive-prefix correctly includes extra chars compared to the
 > initial comment start.  E.g.
 >
 >         ;; - here is the beginning of the paragraph
 >         ;;   where the fill-prefix will include 3 spaces after ";;"
 >         ;;   even though the first line doesn't match it.
 >
 > It could even be things like
 >
 >         ;; 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 (at
line-beginning)?  If the prefix passed to `comment-valid-prefix-p'
matches the string at compos there shouldn't be any problems.  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.

 > 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?

 >>+   (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.