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

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

[Basil L. Contovounesios]

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

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

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Update-example-major-mode-code-in-Elisp-manual.patch --]
[-- Type: text/x-diff, Size: 3792 bytes --]

From b232d11d6753a98563badc823668214ff32e6d81 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Tue, 26 Feb 2019 11:57:53 +0000
Subject: [PATCH 1/3] Update example major mode code in Elisp manual

* doc/lispref/modes.texi (Example Major Modes): Update code examples
to reflect current state of lisp/textmodes/text-mode.el and
lisp/emacs-lisp/lisp-mode.el. (bug#34671)
---
 doc/lispref/modes.texi | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 6349dec98b..f4d7f41a62 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -1237,6 +1237,7 @@ Example Major Modes
     (modify-syntax-entry ?\\ ".   " st)
     ;; Add 'p' so M-c on 'hello' leads to 'Hello', not 'hello'.
     (modify-syntax-entry ?' "w p" st)
+    @dots{}
     st)
   "Syntax table used while in `text-mode'.")
 @end group
@@ -1246,6 +1247,7 @@ Example Major Modes
 (defvar text-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\e\t" 'ispell-complete-word)
+    @dots{}
     map)
   "Keymap for `text-mode'.
 Many other modes, such as `mail-mode', `outline-mode' and
@@ -1289,11 +1291,11 @@ Example Major Modes
 @smallexample
 @group
 ;; @r{Create mode-specific table variables.}
-(defvar lisp-mode-abbrev-table nil)
-(define-abbrev-table 'lisp-mode-abbrev-table ())
+(define-abbrev-table 'lisp-mode-abbrev-table ()
+  "Abbrev table for Lisp mode.")
 
 (defvar lisp-mode-syntax-table
-  (let ((table (copy-syntax-table emacs-lisp-mode-syntax-table)))
+  (let ((table (make-syntax-table lisp--mode-syntax-table)))
     (modify-syntax-entry ?\[ "_   " table)
     (modify-syntax-entry ?\] "_   " table)
     (modify-syntax-entry ?# "' 14" table)
@@ -1308,10 +1310,9 @@ Example Major Modes
 
 @smallexample
 @group
-(defun lisp-mode-variables (&optional syntax keywords-case-insensitive)
+(defun lisp-mode-variables (&optional syntax keywords-case-insensitive elisp)
   (when syntax
     (set-syntax-table lisp-mode-syntax-table))
-  (setq local-abbrev-table lisp-mode-abbrev-table)
   @dots{}
 @end group
 @end smallexample
@@ -1322,8 +1323,7 @@ Example Major Modes
 
 @smallexample
 @group
-  (make-local-variable 'comment-start)
-  (setq comment-start ";")
+  (setq-local comment-start ";")
   @dots{}
 @end group
 @end smallexample
@@ -1337,6 +1337,7 @@ Example Major Modes
 @group
 (defvar lisp-mode-shared-map
   (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map prog-mode-map)
     (define-key map "\e\C-q" 'indent-sexp)
     (define-key map "\177" 'backward-delete-char-untabify)
     map)
@@ -1351,7 +1352,7 @@ Example Major Modes
 @group
 (defvar lisp-mode-map
   (let ((map (make-sparse-keymap))
-	(menu-map (make-sparse-keymap "Lisp")))
+        (menu-map (make-sparse-keymap "Lisp")))
     (set-keymap-parent map lisp-mode-shared-map)
     (define-key map "\e\C-x" 'lisp-eval-defun)
     (define-key map "\C-c\C-z" 'run-lisp)
@@ -1375,17 +1376,13 @@ Example Major Modes
 
 \\@{lisp-mode-map@}
 Note that `run-lisp' may be used either to start an inferior Lisp job
-or to switch back to an existing one.
+or to switch back to an existing one."
 @end group
-
 @group
-Entry to this mode calls the value of `lisp-mode-hook'
-if that value is non-nil."
   (lisp-mode-variables nil t)
-  (set (make-local-variable 'find-tag-default-function)
-       'lisp-find-tag-default)
-  (set (make-local-variable 'comment-start-skip)
-       "\\(\\(^\\|[^\\\\\n]\\)\\(\\\\\\\\\\)*\\)\\(;+\\|#|\\) *")
+  (setq-local find-tag-default-function 'lisp-find-tag-default)
+  (setq-local comment-start-skip
+              "\\(\\(^\\|[^\\\\\n]\\)\\(\\\\\\\\\\)*\\)\\(;+\\|#|\\) *")
   (setq imenu-case-fold-search t))
 @end group
 @end smallexample
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Use-lexical-binding-in-text-mode.el.patch --]
[-- Type: text/x-diff, Size: 8057 bytes --]

From ac2a834bcd6d0474107c439800a9ec66adff16eb Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Tue, 26 Feb 2019 14:22:32 +0000
Subject: [PATCH 2/3] Use lexical-binding in text-mode.el

* lisp/textmodes/text-mode.el: Use lexical-binding.
(text-mode-syntax-table): Align comments to comment-column.
(text-mode-map, text-mode): Refill docstring.
(text-mode, paragraph-indent-minor-mode, text-mode-hook-identify):
Use setq-local.
(toggle-text-mode-auto-fill): Hoist save-current-buffer out of loop.
(center-region, center-line): Tiny simplification.

* doc/lispref/modes.texi (Example Major Modes): Adapt code examples
to these text-mode.el changes.
---
 doc/lispref/modes.texi      | 16 +++++-----
 lisp/textmodes/text-mode.el | 60 ++++++++++++++++++-------------------
 2 files changed, 37 insertions(+), 39 deletions(-)

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index f4d7f41a62..44efff416e 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -1261,17 +1261,17 @@ Example Major Modes
 @group
 (define-derived-mode text-mode nil "Text"
   "Major mode for editing text written for humans to read.
-In this mode, paragraphs are delimited only by blank or white lines.
-You can thus get the full benefit of adaptive filling
- (see the variable `adaptive-fill-mode').
-\\@{text-mode-map@}
+In this mode, paragraphs are delimited only by blank or white
+lines.  You can thus get the full benefit of adaptive
+filling (see the variable `adaptive-fill-mode').
+
+\\{text-mode-map}
 Turning on Text mode runs the normal hook `text-mode-hook'."
 @end group
 @group
-  (set (make-local-variable 'text-mode-variant) t)
-  (set (make-local-variable 'require-final-newline)
-       mode-require-final-newline)
-  (set (make-local-variable 'indent-line-function) 'indent-relative))
+  (setq-local text-mode-variant t)
+  (setq-local require-final-newline mode-require-final-newline)
+  (setq-local indent-line-function #'indent-relative))
 @end group
 @end smallexample
 
diff --git a/lisp/textmodes/text-mode.el b/lisp/textmodes/text-mode.el
index 931faadb5b..8dcc4e3696 100644
--- a/lisp/textmodes/text-mode.el
+++ b/lisp/textmodes/text-mode.el
@@ -1,4 +1,4 @@
-;;; text-mode.el --- text mode, and its idiosyncratic commands
+;;; text-mode.el --- text mode, and its idiosyncratic commands  -*- lexical-binding: t -*-
 
 ;; Copyright (C) 1985, 1992, 1994, 2001-2019 Free Software Foundation,
 ;; Inc.
@@ -49,7 +49,7 @@ text-mode-syntax-table
     (modify-syntax-entry ?' "w p" st)
     ;; UAX #29 says HEBREW PUNCTUATION GERESH behaves like a letter
     ;; for the purposes of finding word boundaries.
-    (modify-syntax-entry #x5f3 "w   ") ; GERESH
+    (modify-syntax-entry #x5f3 "w   ")  ; GERESH
     ;; UAX #29 says HEBREW PUNCTUATION GERSHAYIM should not be a word
     ;; boundary when surrounded by letters.  Our infrastructure for
     ;; finding a word boundary doesn't support 3-character
@@ -57,13 +57,13 @@ text-mode-syntax-table
     ;; character.  This leaves a problem of having GERSHAYIM at the
     ;; beginning or end of a word, where it should be a boundary;
     ;; FIXME.
-    (modify-syntax-entry #x5f4 "w   ") ; GERSHAYIM
+    (modify-syntax-entry #x5f4 "w   ")  ; GERSHAYIM
     ;; These all should not be a word boundary when between letters,
     ;; according to UAX #29, so they again are prone to the same
     ;; problem as GERSHAYIM; FIXME.
-    (modify-syntax-entry #xb7 "w   ")	; MIDDLE DOT
-    (modify-syntax-entry #x2027 "w   ")	; HYPHENATION POINT
-    (modify-syntax-entry #xff1a "w   ")	; FULLWIDTH COLON
+    (modify-syntax-entry #xb7 "w   ")   ; MIDDLE DOT
+    (modify-syntax-entry #x2027 "w   ") ; HYPHENATION POINT
+    (modify-syntax-entry #xff1a "w   ") ; FULLWIDTH COLON
     st)
   "Syntax table used while in `text-mode'.")
 
@@ -93,21 +93,21 @@ text-mode-map
                   :help "Center the current line"))
     map)
   "Keymap for `text-mode'.
-Many other modes, such as `mail-mode', `outline-mode' and `indented-text-mode',
-inherit all the commands defined in this map.")
+Many other modes, such as `mail-mode', `outline-mode', and
+`indented-text-mode', inherit all the commands defined in this map.")
 
 \f
 (define-derived-mode text-mode nil "Text"
   "Major mode for editing text written for humans to read.
-In this mode, paragraphs are delimited only by blank or white lines.
-You can thus get the full benefit of adaptive filling
- (see the variable `adaptive-fill-mode').
+In this mode, paragraphs are delimited only by blank or white
+lines.  You can thus get the full benefit of adaptive
+filling (see the variable `adaptive-fill-mode').
+
 \\{text-mode-map}
 Turning on Text mode runs the normal hook `text-mode-hook'."
-  (set (make-local-variable 'text-mode-variant) t)
-  (set (make-local-variable 'require-final-newline)
-       mode-require-final-newline)
-  (set (make-local-variable 'indent-line-function) 'indent-relative))
+  (setq-local text-mode-variant t)
+  (setq-local require-final-newline mode-require-final-newline)
+  (setq-local indent-line-function #'indent-relative))
 
 (define-derived-mode paragraph-indent-text-mode text-mode "Parindent"
   "Major mode for editing text, with leading spaces starting a paragraph.
@@ -134,11 +134,10 @@ paragraph-indent-minor-mode
     (if (eq t (compare-strings ps-re nil nil
                                paragraph-start nil (length ps-re)))
         (if (not paragraph-indent-minor-mode)
-            (set (make-local-variable 'paragraph-start)
-                 (substring paragraph-start (length ps-re))))
+            (setq-local paragraph-start
+                        (substring paragraph-start (length ps-re))))
       (if paragraph-indent-minor-mode
-          (set (make-local-variable 'paragraph-start)
-               (concat ps-re paragraph-start)))))
+          (setq-local paragraph-start (concat ps-re paragraph-start)))))
   ;; Change the indentation function.
   (if paragraph-indent-minor-mode
       (add-function :override (local 'indent-line-function)
@@ -154,7 +153,7 @@ 'indented-text-mode
 (defun text-mode-hook-identify ()
   "Mark that this mode has run `text-mode-hook'.
 This is how `toggle-text-mode-auto-fill' knows which buffers to operate on."
-  (set (make-local-variable 'text-mode-variant) t))
+  (setq-local text-mode-variant t))
 
 (defun toggle-text-mode-auto-fill ()
   "Toggle whether to use Auto Fill in Text mode and related modes.
@@ -163,10 +162,11 @@ toggle-text-mode-auto-fill
   (interactive)
   (let ((enable-mode (not (memq 'turn-on-auto-fill text-mode-hook))))
     (if enable-mode
-	(add-hook 'text-mode-hook 'turn-on-auto-fill)
-      (remove-hook 'text-mode-hook 'turn-on-auto-fill))
-    (dolist (buffer (buffer-list))
-      (with-current-buffer buffer
+        (add-hook 'text-mode-hook #'turn-on-auto-fill)
+      (remove-hook 'text-mode-hook #'turn-on-auto-fill))
+    (save-current-buffer
+      (dolist (buffer (buffer-list))
+        (set-buffer buffer)
 	(if (or (derived-mode-p 'text-mode) text-mode-variant)
 	    (auto-fill-mode (if enable-mode 1 0)))))
     (message "Auto Fill %s in Text modes"
@@ -191,8 +191,7 @@ center-region
 See `center-line' for more info."
   (interactive "r")
   (if (> from to)
-      (let ((tem to))
-	(setq to from from tem)))
+      (setq to (prog1 from (setq from to))))
   (save-excursion
     (save-restriction
       (narrow-to-region from to)
@@ -214,15 +213,14 @@ center-line
   (while (not (eq nlines 0))
     (save-excursion
       (let ((lm (current-left-margin))
-	    line-length)
+            space)
 	(beginning-of-line)
 	(delete-horizontal-space)
 	(end-of-line)
 	(delete-horizontal-space)
-	(setq line-length (current-column))
-	(if (> (- fill-column lm line-length) 0)
-	    (indent-line-to
-	     (+ lm (/ (- fill-column lm line-length) 2))))))
+        (setq space (- fill-column lm (current-column)))
+        (if (> space 0)
+            (indent-line-to (+ lm (/ space 2))))))
     (cond ((null nlines)
 	   (setq nlines 0))
 	  ((> nlines 0)
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0003-Do-not-set-indent-line-function-in-text-mode.patch --]
[-- Type: text/x-diff, Size: 2038 bytes --]

From 7c655b0f0363134f750bae25f45198b713f851b7 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Tue, 26 Feb 2019 16:13:23 +0000
Subject: [PATCH 3/3] Do not set indent-line-function in text-mode

* lisp/textmodes/text-mode.el (text-mode): Remove redundant setting
of indent-line-function to indent-relative, which is its default
value.

* doc/lispref/modes.texi (Example Major Modes): Adapt docs
accordingly.
---
 doc/lispref/modes.texi      | 7 +------
 lisp/textmodes/text-mode.el | 3 +--
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi
index 44efff416e..08d96e6a65 100644
--- a/doc/lispref/modes.texi
+++ b/doc/lispref/modes.texi
@@ -1270,15 +1270,10 @@ Example Major Modes
 @end group
 @group
   (setq-local text-mode-variant t)
-  (setq-local require-final-newline mode-require-final-newline)
-  (setq-local indent-line-function #'indent-relative))
+  (setq-local require-final-newline mode-require-final-newline))
 @end group
 @end smallexample
 
-@noindent
-(The last line is redundant nowadays, since @code{indent-relative} is
-the default value, and we'll delete it in a future version.)
-
 @cindex @file{lisp-mode.el}
   The three Lisp modes (Lisp mode, Emacs Lisp mode, and Lisp Interaction
 mode) have more features than Text mode and the code is correspondingly
diff --git a/lisp/textmodes/text-mode.el b/lisp/textmodes/text-mode.el
index 8dcc4e3696..9f3cdf46e0 100644
--- a/lisp/textmodes/text-mode.el
+++ b/lisp/textmodes/text-mode.el
@@ -106,8 +106,7 @@ text-mode
 \\{text-mode-map}
 Turning on Text mode runs the normal hook `text-mode-hook'."
   (setq-local text-mode-variant t)
-  (setq-local require-final-newline mode-require-final-newline)
-  (setq-local indent-line-function #'indent-relative))
+  (setq-local require-final-newline mode-require-final-newline))
 
 (define-derived-mode paragraph-indent-text-mode text-mode "Parindent"
   "Major mode for editing text, with leading spaces starting a paragraph.
-- 
2.20.1


[-- Attachment #4: Type: text/plain, Size: 777 bytes --]


"Basil L. Contovounesios" <contovob@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

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Tue, 26 Feb 2019 17:06:58 +0000
> 
> "Basil L. Contovounesios" <contovob@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?

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

[Basil L. Contovounesios]

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

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.

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

[Basil L. Contovounesios]

tags 34671 fixed
close 34671 26.2
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: <34671@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

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

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: <34671-done@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.