bug#34671: 27.0.50; Outdated code listings in (elisp) Example Major Modes

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: <34671@debbugs.gnu.org>
> Date: Fri, 01 Mar 2019 19:46:31 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The first patch reconciles the code listings in the manual with the
> >> current state of the corresponding libraries.
> >
> > This part is definitely OK, please push to the emacs-26 branch.
> 
> Sorry, I don't have write access, so someone else will have to do that.

Done.

> BTW, is one expected to be a package author before it is acceptable to
> request write access, or is it a matter of amassing enough flight hours?

The latter.  I think you are there already, so feel free to create a
user on Savannah and then request write access.

> >> The second patch enables lexical-binding in text-mode.el along with some
> >> minor aesthetic changes.
> >
> > I don't see the need for parts of this patch.  Enabling
> > lexical-binding is OK, but it should be a separate change unrelated to
> > this bug report.
> 
> Should I submit a new bug report for that, or message emacs-devel?

The latter, I think, since the main issue here is how well was the
package tested after turning on lexical-binding.

> > And I'm not sure I see the reason for the other changes, nor even
> > agree with them.  In particular, please modify whitespace only where
> > you actually make other non-trivial changes.
> 
> (Oops, looking back I accidentally mangled some Texinfo escape
> characters by copy-pasting.)
> 
> I agree with this principle, but simply thought that a file-wide change
> like enabling lexical-binding was sufficient excuse for minor "cleanups"
> along the way.

Not in such a sweeping manner.  In general, only in the same
function/code fragment where you are making changes.

> I assume the parts of the patch you're least keen on are:
> 
> * lisp/textmodes/text-mode.el (text-mode-syntax-table): Align comments
> to comment-column.
> (toggle-text-mode-auto-fill): Hoist save-current-buffer out of loop.
> (center-region): Tiny simplification.
> 
> but that you're okay with the following:
> 
> * lisp/textmodes/text-mode.el: Use lexical-binding.
> (text-mode-map, text-mode): Refill docstring.
> (text-mode, paragraph-indent-minor-mode, text-mode-hook-identify):
> Use setq-local.
> (center-line): Tiny simplification.

Refilling doc strings also caused me to raise a brow.  IMO, that is
only justified if the original doc string is badly formatted, like has
overly-long lines, close to or longer than 80 columns.  Otherwise, we
don't generally refill doc strings we don't change, we only make sure
they look well and read clearly on the screen.

> >> The last patch fulfils an old promise in the manual to eventually forgo
> >> setting indent-line-function in text-mode, which is considered
> >> redundant.
> >
> > What if the user customizes the default values, shouldn't text-mode
> > reset that in the buffer where it is turned on?
> 
> I can think of two reasons for keeping the reset in text-mode:
> 
> 0. If text-mode breaks when indent-line-function is set to anything
>    other than indent-relative.
> 
>    I'm not sure, but I don't think this is the case, as I set the
>    variable in text-mode-hook for a while without any noticeable
>    fallout.
> 
> 1. To avoid a user-visible change of behaviour with user configurations
>    (like mine) which change the default global value of
>    indent-line-function.
> 
> The only reason (I can think of) to remove the reset is in order to
> fulfil the relevant promise in the manual.
> 
> Keeping the reset is clearly less risky.  I don't mind either way, so
> long as the side-note in the manual accurately reflects future
> intentions.

Maybe we should make the change you propose, but it isn't clear-cut
and requires discussion in a separate bug report, or perhaps even on
emacs-devel.  It certainly isn't cleanup.

Thanks.