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

[Stefan Monnier]

> Why?  compos is at the beginning of the current comment provided `point'
> is within a comment.

Oh right, I'm not making sense.  Then how 'bout the 100% guaranteed untested
patch below.

> The problem that `comment-indent-new-line' used
> the fill-prefix in the "not compos" case is trivially handled in the
> "else" part of `comment-valid-prefix-p'.  The problem you refer to is
> with the "then" part.

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

> Well, checking comment-end style is cheaper than doing comment-forward.
> Combine these before doing the string-match?

But the code *already* does the comment-forward check.  As I pointed out
your code does the same thing as mine, just slightly differently.  This part
of the different seems to be just "worse".  The other part OTOH seems to be
"partly better".
Please merge them.

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

> ... but the string at compos is ";; >" ...

Not necessarily.  I may compute this in adaptive-fill-function straight from
the first line, (when using auto-fill on the first line).

>>> (at line-beginning)?
>> These can happen at any indentation.
> ... if compos is not at line beginning the prefix is rejected.

Where?  Why?

> The only cases rejected are when the string is not at line beginning or
> does not match the prefix.  In your second example the prefix is ";; ",
> at least with emacs -Q, because that's the common prefix of ";; " and
> ";; >".

But your code will be used even if "-Q" is not passed.

>>> 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 fail to understand you here.  If the comment at compos doesn't match
> the prefix why should I want to insert the prefix on the next line?

Why not?

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

> It fixes that better than I expected.  Note also that you never had a
> chance to handle this _before_ the introduction of the doc-string face.
> But I admit that emacs was always breaking lisp strings this way, hence
> let's drop it (which means I don't need to change fill.el either).

>> Have you tried
>> comment-auto-fill-only-comments?

> It doesn't fill doc-strings.

But maybe it's the right place to introduce such a feature.
Basically extend this var so you can say "fill in FOO" where FOO is the list
of possible contexts, such as `comment', `string', `doc', `code'?


        Stefan


--- newcomment.el       24 aoû 2006 14:41:55 -0400      1.96
+++ newcomment.el       08 oct 2006 21:33:15 -0400      
@@ -1124,12 +1124,44 @@
   :group 'comment)
 
 (defun comment-valid-prefix-p (prefix compos)
+  "Check that the adaptive-fill-prefix is consistent with the context.
+PREFIX is the prefix (presumably guessed by `adaptive-fill-mode').
+COMPOS is the position of the beginning of the comment we're in,
+or nil if we're not inside a comment."
+  ;; This consistency checking is mostly needed to workaround the limitation
+  ;; of auto-fill-mode whose paragraph-determination doesn't pay attention
+  ;; to comment boundaries.
+  (if (null compos)
+      ;; We're not inside a comment: the prefix shouldn't match
+      ;; a comment-starter.
+      (not (and comment-start comment-start-skip
+                (string-match comment-start-skip prefix)))
   (or
    ;; Accept any prefix if the current comment is not EOL-terminated.
    (save-excursion (goto-char compos) (comment-forward) (not (bolp)))
-   ;; Accept any prefix that starts with a comment-start marker.
-   (string-match (concat "\\`[ \t]*\\(?:" comment-start-skip "\\)")
-                prefix)))
+     ;; Accept any prefix that starts with the same comment-start marker
+     ;; as the current one.
+     (when (string-match (concat "\\`[ \t]*\\(?:" comment-start-skip "\\)")
+                         prefix)
+       (let ((prefix-com (comment-string-strip (match-string 0) nil t)))
+         (string-match "\\`[ \t]*" prefix-com)
+         (let* ((prefix-space (match-string 0))
+                (prefix-indent (string-width prefix-space))
+                (prefix-comstart (substring prefix-com (match-end 0))))
+           (save-excursion
+             (goto-char compos)
+             ;; The comstart marker is the same.
+             (and (looking-at (regexp-quote prefix-comstart))
+                  ;; The indentation as well.
+                  (or (= prefix-indent
+                         (- (current-column) (current-left-margin)))
+                      ;; Check the indentation in two different ways, just
+                      ;; to try and avoid most of the potential funny cases.
+                      (equal prefix-space
+                             (buffer-substring (point)
+                                               (progn (move-to-left-margin)
+                                                      (point)))))))))))))
+                    
 
 ;;;###autoload
 (defun comment-indent-new-line (&optional soft)
@@ -1182,8 +1214,7 @@
         ;; If there's an adaptive prefix, use it unless we're inside
         ;; a comment and the prefix is not a comment starter.
         ((and fill-prefix
-              (or (not compos)
-                  (comment-valid-prefix-p fill-prefix compos)))
+               (comment-valid-prefix-p fill-prefix compos))
          (indent-to-left-margin)
          (insert-and-inherit fill-prefix))
         ;; If we're not inside a comment, just try to indent.