GNU bug report logs - #34671
27.0.50; Outdated code listings in (elisp) Example Major Modes

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 34671 in the body.
You can then email your comments to 34671 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Outdated code listings in (elisp) Example Major Modes
Date: Tue, 26 Feb 2019 16:57:12 +0000

Some of the examples in '(elisp) Example Major Modes' do not reflect the
current code in lisp/textmodes/text-mode.el and
lisp/emacs-lisp/lisp-mode.el.  Furthermore, the indentation of the
lisp-mode-map listing in the manual is off due to a tab character in its
Texinfo source.

Patch(es) to follow.

-- 
Basil

In GNU Emacs 27.0.50 (build 9, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2019-02-25 built on thunk
Repository revision: 0d49078ad80f54b810180a071e2b6b4bcc024851
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12003000
System Description: Debian GNU/Linux buster/sid

Message #8 received at 34671 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: 34671 <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Tue, 26 Feb 2019 17:06:58 +0000

[0001-Update-example-major-mode-code-in-Elisp-manual.patch (text/x-diff, attachment)]

[0002-Use-lexical-binding-in-text-mode.el.patch (text/x-diff, attachment)]

[0003-Do-not-set-indent-line-function-in-text-mode.patch (text/x-diff, attachment)]

[Message part 4 (text/plain, inline)]

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Some of the examples in '(elisp) Example Major Modes' do not reflect the
> current code in lisp/textmodes/text-mode.el and
> lisp/emacs-lisp/lisp-mode.el.  Furthermore, the indentation of the
> lisp-mode-map listing in the manual is off due to a tab character in its
> Texinfo source.
>
> Patch(es) to follow.

I attach said patches; WDYT?

The first patch reconciles the code listings in the manual with the
current state of the corresponding libraries.

The second patch enables lexical-binding in text-mode.el along with some
minor aesthetic changes.

The last patch fulfils an old promise in the manual to eventually forgo
setting indent-line-function in text-mode, which is considered
redundant.

Thanks,

-- 
Basil

Message #11 received at 34671 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 34671 <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Fri, 01 Mar 2019 12:15:48 +0200

> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Date: Tue, 26 Feb 2019 17:06:58 +0000
> 
> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
> 
> > Some of the examples in '(elisp) Example Major Modes' do not reflect the
> > current code in lisp/textmodes/text-mode.el and
> > lisp/emacs-lisp/lisp-mode.el.  Furthermore, the indentation of the
> > lisp-mode-map listing in the manual is off due to a tab character in its
> > Texinfo source.
> >
> > Patch(es) to follow.
> 
> I attach said patches; WDYT?

Thanks.

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

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

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

Message #14 received at 34671 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 34671 <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Fri, 01 Mar 2019 19:46:31 +0000

Eli Zaretskii <eliz <at> 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.
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 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?

> 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.  I'm sorry the proposed changes weren't clean enough.

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.

Let me know and I will send updated patch(es) to a new ticket/thread.

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

Thanks,

-- 
Basil

Message #17 received at 34671 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 34671 <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Sat, 02 Mar 2019 09:39:27 +0200

> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Cc: <34671 <at> debbugs.gnu.org>
> Date: Fri, 01 Mar 2019 19:46:31 +0000
> 
> Eli Zaretskii <eliz <at> 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.

Message #24 received at 34671-done <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 34671-done <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Sat, 02 Mar 2019 11:36:51 +0000

tags 34671 fixed
close 34671 26.2
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
>> Cc: <34671 <at> debbugs.gnu.org>
>> Date: Fri, 01 Mar 2019 19:46:31 +0000
>> 
>> Sorry, I don't have write access, so someone else will have to do that.
>
> Done.

Thanks.

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

Thanks, will do (on both counts).

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

Fair enough, will be more careful in future.  In my defence, I only
refilled these two docstrings after adding an Oxford comma to
text-mode-map and in order to avoid a leading space in text-mode.

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

Agreed, will ask elsewhere, so I'm closing this ticket.
Sorry for the hassle.

Thanks,

-- 
Basil

Message #27 received at 34671 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 34671 <at> debbugs.gnu.org
Subject: Re: bug#34671: 27.0.50;
 Outdated code listings in (elisp) Example Major Modes
Date: Sat, 02 Mar 2019 14:23:39 +0200

> From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
> Cc: <34671-done <at> debbugs.gnu.org>
> Date: Sat, 02 Mar 2019 11:36:51 +0000
> 
> Sorry for the hassle.

No need to apologize, you did nothing that requires an apology.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.