bug#31656: 26.1; `fill-paragraph' malformats in emacs-lisp-mode

[Stefan Guath <stefan@automata.se>]

[-- Attachment #1: Type: text/plain, Size: 4801 bytes --]

The very idea of shadowing the global `fill-column' in the first place
breaks the Principle Of Least Astonishment and introduces annoyance and
extra work for the user when trying to understand why the explicit setting
of `fill-column' is not respected. If it is to be done anyway, it needs to
be well motivated and also executed in a correct way.

So, I guess my basic question is what value
`emacs-lisp-docstring-fill-column' tries to add? It's obviously not aiming
to introduce a general shadowing for elisp files only, since that mechanism
is already present due to `fill-column' being buffer local. Rather,
according to its docs, it tries to add selective shadowing for elisp-files
in doc strings only. Fair enough, that is an edge case where a shadowing
mechanism could be motivated. But instead it implements some kind of
unpredictable shadowing - I don't even understand in what exact
circumstances the shadowing takes place. If we were to just update its
docs, what should it be updated to?

To me this entire functionality should either be removed or fixed so that
its behavior reflects its current docs. In its current state it just
malformats elisp files in unexpected ways. Or am I missing something?


On Fri, Jun 1, 2018 at 11:39 AM Noam Postavsky <npostavs@gmail.com> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Stefan Guath <stefan@automata.se>
> >> Date: Wed, 30 May 2018 14:50:01 +0200
> >>
> >> `emacs-lisp-docstring-fill-column' shadows `fill-column' in too many
> cases in emacs-lisp-mode. The
> >> documentation of `emacs-lisp-docstring-fill-column' states: "Value of
> ‘fill-column’ to use when filling a
> >> docstring". But it incorrectly seems to be used in a lot more cases
> than just in a docstring (the only case that
> >> I've found where ‘fill-column’ is actually respected is within
> comments). A work-around is to set
> >> `emacs-lisp-docstring-fill-column' to nil, but it would be nice to have
> it working properly instead.
> >>
> >> I might be missing something, but think the incorrect behavior is to be
> found in `lisp-fill-paragraph' that is
> >> invoked by `fill-paragraph' through `fill-paragraph-function'. It seems
> like `lisp-fill-paragraph' unconditionally
> >> sets `fill-column' to `emacs-lisp-docstring-fill-column' without
> checking whether point is within a doc string
> >> first. The only requirements for enable shadowing currently seems to be
> "(and (integerp
> >> emacs-lisp-docstring-fill-column) (derived-mode-p 'emacs-lisp-mode))",
> which doesn't seems sufficient.
> >
> > AFAICT, this behavior was in Emacs since about forever (since 1995, t
> > be precise).  So maybe we just need to adjust the doc string to
> > reflect the reality?
> >
> > Or are there real-life use cases where this behavior is grossly
> > inappropriate?
>
> I don't think it makes sense to apply normal plain text filling rules to
> code.  Maybe it doesn't come up much because people don't usually call
> M-q on code, and usually lines of code are kept short enough that they
> wouldn't get filled anyway.  But picking a random example from rgrep
> ^.\{100,\}$ on the Emacs code base:
>
> (defun feedmail-default-date-generator (maybe-file)
>   "Default function for generating Date: header contents."
>   (feedmail-say-debug ">in-> feedmail-default-date-generator")
>   (when maybe-file
>     (feedmail-say-debug (concat "4 cre " (feedmail-rfc822-date (nth 4
> (file-attributes maybe-file)))))
>     (feedmail-say-debug (concat "5 mod " (feedmail-rfc822-date (nth 5
> (file-attributes maybe-file)))))
>     (feedmail-say-debug (concat "6 sta " (feedmail-rfc822-date (nth 6
> (file-attributes maybe-file))))))
>   (let ((date-time))
>     (if (and (not feedmail-queue-use-send-time-for-date) maybe-file)
>         (setq date-time (nth 5 (file-attributes maybe-file))))
>     (feedmail-rfc822-date date-time))
>   )
>
> Running M-q on every line turns it into this nonsense:
>
> (defun feedmail-default-date-generator (maybe-file)
>   "Default function for generating Date: header contents."
>   (feedmail-say-debug ">in-> feedmail-default-date-generator")
>   (when maybe-file
>     (feedmail-say-debug (concat "4
>     cre " (feedmail-rfc822-date (nth 4 (file-attributes
>     maybe-file)))))
>     (feedmail-say-debug (concat "5
>     mod " (feedmail-rfc822-date (nth 5 (file-attributes
>     maybe-file)))))
>     (feedmail-say-debug (concat "6
>     sta " (feedmail-rfc822-date (nth 6 (file-attributes
>     maybe-file))))))
>   (let ((date-time))
>     (if (and (not feedmail-queue-use-send-time-for-date)
>     maybe-file)
>         (setq date-time (nth 5 (file-attributes maybe-file))))
>     (feedmail-rfc822-date date-time)) )
>

[-- Attachment #2: Type: text/html, Size: 5712 bytes --]