bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

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

The warning about a missing lexical-binding cookie rather belongs in the compiler than checkdoc, because it's not about documentation or style but code generation and ability to detect errors, both which are hindered by a missing cookie.

Moving the warning to the compiler also makes it more widely seen.


[-- Attachment #2: 0001-Move-lexical-binding-warning-from-checkdoc-to-byte-c.patch --]
[-- Type: application/octet-stream, Size: 11207 bytes --]

From 6887fadd1c262954303d706d0d556bef4b5148c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Sun, 15 Oct 2023 22:01:06 +0200
Subject: [PATCH] Move lexical-binding warning from checkdoc to byte-compiler

This warning is much more appropriate for the compiler, since lexical
binding affects what it can reason and warn about, than for checkdoc
as the warning has no bearing to documentation at all.
The move also improves the reach of the warning.

* etc/NEWS: Update.
* lisp/emacs-lisp/checkdoc.el (checkdoc-lexical-binding-flag)
(checkdoc-file-comments-engine): Move warning from here....
* lisp/emacs-lisp/bytecomp.el (byte-compile-file): ...to here.
* test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el:
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp-tests--unescaped-char-literals)
(bytecomp-tests-function-put, bytecomp-tests--not-writable-directory)
(bytecomp-tests--target-file-no-directory):
Update tests.
(bytecomp-tests--log-from-compilation)
(bytecomp-tests--lexical-binding-cookie): New test.
---
 etc/NEWS                                      | 25 ++++++++----
 lisp/emacs-lisp/bytecomp.el                   |  4 ++
 lisp/emacs-lisp/checkdoc.el                   | 39 -------------------
 .../bytecomp-resources/no-byte-compile.el     |  2 +-
 test/lisp/emacs-lisp/bytecomp-tests.el        | 39 +++++++++++++++++--
 5 files changed, 57 insertions(+), 52 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 02b794a2964..e7f51e1c6c4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -845,14 +845,6 @@ This can help avoid some awkward skip conditions.  For example
 '(skip-unless (not noninteractive))' can be changed to the easier
 to read '(skip-when noninteractive)'.
 
-** Checkdoc
-
----
-*** New checkdock warning if not using lexical-binding.
-Checkdoc now warns if the first line of an Emacs Lisp file does not
-end with a "-*- lexical-binding: t -*-" cookie.  Customize the user
-option 'checkdoc-lexical-binding-flag' to nil to disable this warning.
-
 ** URL
 
 +++
@@ -1153,6 +1145,23 @@ sexp-related motion commands.
 
 ** New or changed byte-compilation warnings
 
+---
+*** Warn about missing 'lexical-binding' directive.
+The compiler now warns if an Elisp file lacks the standard
+'-*- lexical-binding: ... -*-' cookie on the first line.
+This line typically looks something like
+
+    ;;; My little pony mode  -*- lexical-binding: t -*-
+
+It is needed to inform the compiler about which dialect of ELisp
+your code is using: the modern dialect with lexical binding or
+the old dialect with only dynamic binding.
+
+Lexical binding avoids some name conflicts and allows the compiler
+to detect more mistakes and generate more efficient code.  To adapt
+your code to lexical binding, see the "(elisp) Converting to Lexical
+Binding" node in the manual.
+
 ---
 *** Warn about empty bodies for more special forms and macros.
 The compiler now warns about an empty body argument to 'when',
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 92abe6b4624..cc68db73c9f 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2201,6 +2201,10 @@ byte-compile-file
               filename buffer-file-name))
       ;; Don't inherit lexical-binding from caller (bug#12938).
       (unless (local-variable-p 'lexical-binding)
+        (let ((byte-compile-current-buffer (current-buffer)))
+          (byte-compile-warn-x
+           (position-symbol 'a (point-min))
+           "file has no `lexical-binding' directive on its first line"))
         (setq-local lexical-binding nil))
       ;; Set the default directory, in case an eval-when-compile uses it.
       (setq default-directory (file-name-directory filename)))
diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 440e133f44b..471a2fbdf48 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -128,14 +128,6 @@
 ;; simple style rules to follow which checkdoc will auto-fix for you.
 ;; `y-or-n-p' and `yes-or-no-p' should also end in "?".
 ;;
-;; Lexical binding:
-;;
-;;   We recommend always using lexical binding in new code, and
-;; converting old code to use it.  Checkdoc warns if you don't have
-;; the recommended string "-*- lexical-binding: t -*-" at the top of
-;; the file.  You can disable this check with the user option
-;; `checkdoc-lexical-binding-flag'.
-;;
 ;; Adding your own checks:
 ;;
 ;;   You can experiment with adding your own checks by setting the
@@ -347,12 +339,6 @@ checkdoc-column-zero-backslash-before-paren
   :type 'boolean
   :version "28.1")
 
-(defcustom checkdoc-lexical-binding-flag t
-  "Non-nil means generate warnings if file is not using lexical binding.
-See Info node `(elisp) Converting to Lexical Binding' for more."
-  :type 'boolean
-  :version "30.1")
-
 ;; This is how you can use checkdoc to make mass fixes on the Emacs
 ;; source tree:
 ;;
@@ -2391,31 +2377,6 @@ checkdoc-file-comments-engine
 	      (point-min) (save-excursion (goto-char (point-min))
 					  (line-end-position))))
 	 nil))
-      (when checkdoc-lexical-binding-flag
-        (setq
-         err
-         ;; Lexical binding cookie.
-         (if (not (save-excursion
-                    (save-restriction
-                      (goto-char (point-min))
-                      (narrow-to-region (point) (pos-eol))
-                      (re-search-forward
-                       (rx "-*-" (* (* nonl) ";")
-                           (* space) "lexical-binding:" (* space) "t" (* space)
-                           (* ";" (* nonl))
-                           "-*-")
-                       nil t))))
-             (let ((pos (save-excursion (goto-char (point-min))
-                                        (goto-char (pos-eol))
-                                        (point))))
-               (if (checkdoc-y-or-n-p "There is no lexical-binding cookie!  Add one?")
-                   (progn
-                     (goto-char pos)
-                     (insert "  -*- lexical-binding: t -*-"))
-                 (checkdoc-create-error
-                  "The first line should end with \"-*- lexical-binding: t -*-\""
-                  pos (1+ pos) t)))
-           nil)))
       (setq
        err
        (or
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
index 00ad1947507..1de5cf66b66 100644
--- a/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
+++ b/test/lisp/emacs-lisp/bytecomp-resources/no-byte-compile.el
@@ -1 +1 @@
-;; -*- no-byte-compile: t; -*-
+;; -*- no-byte-compile: t; lexical-binding: t; -*-
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index e644417c3d4..4aa555f1e92 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1302,6 +1302,30 @@ bytecomp-tests--with-temp-file
        (let ((elc (concat ,file-name-var ".elc")))
          (if (file-exists-p elc) (delete-file elc))))))
 
+(defun bytecomp-tests--log-from-compilation (source)
+  "Compile the string SOURCE and return the compilation log output."
+  (let ((text-quoting-style 'grave)
+        (byte-compile-log-buffer (generate-new-buffer " *Compile-Log*")))
+    (with-current-buffer byte-compile-log-buffer
+      (let ((inhibit-read-only t)) (erase-buffer)))
+    (bytecomp-tests--with-temp-file el-file
+      (write-region source nil el-file)
+      (byte-compile-file el-file))
+    (with-current-buffer byte-compile-log-buffer
+      (buffer-string))))
+
+(ert-deftest bytecomp-tests--lexical-binding-cookie ()
+  (cl-flet ((cookie-warning (source)
+              (string-search
+               "file has no `lexical-binding' directive on its first line"
+               (bytecomp-tests--log-from-compilation source))))
+    (let ((some-code "(defun my-fun () 12)\n"))
+      (should-not (cookie-warning
+                   (concat ";;; -*-lexical-binding:t-*-\n" some-code)))
+      (should-not (cookie-warning
+                   (concat ";;; -*-lexical-binding:nil-*-\n" some-code)))
+      (should (cookie-warning some-code)))))
+
 (ert-deftest bytecomp-tests--unescaped-char-literals ()
   "Check that byte compiling warns about unescaped character
 literals (Bug#20852)."
@@ -1310,7 +1334,9 @@ bytecomp-tests--unescaped-char-literals
         (byte-compile-debug t)
         (text-quoting-style 'grave))
     (bytecomp-tests--with-temp-file source
-      (write-region "(list ?) ?( ?; ?\" ?[ ?])" nil source)
+      (write-region (concat ";;; -*-lexical-binding:t-*-\n"
+                            "(list ?) ?( ?; ?\" ?[ ?])")
+                    nil source)
       (bytecomp-tests--with-temp-file destination
         (let* ((byte-compile-dest-file-function (lambda (_) destination))
                (err (should-error (byte-compile-file source))))
@@ -1322,7 +1348,9 @@ bytecomp-tests--unescaped-char-literals
                                     "`?\\]' expected!")))))))
     ;; But don't warn in subsequent compilations (Bug#36068).
     (bytecomp-tests--with-temp-file source
-      (write-region "(list 1 2 3)" nil source)
+      (write-region (concat ";;; -*-lexical-binding:t-*-\n"
+                            "(list 1 2 3)")
+                    nil source)
       (bytecomp-tests--with-temp-file destination
         (let ((byte-compile-dest-file-function (lambda (_) destination)))
           (should (byte-compile-file source)))))))
@@ -1330,6 +1358,7 @@ bytecomp-tests--unescaped-char-literals
 (ert-deftest bytecomp-tests-function-put ()
   "Check `function-put' operates during compilation."
   (bytecomp-tests--with-temp-file source
+    (insert  ";;; -*-lexical-binding:t-*-\n")
     (dolist (form '((function-put 'bytecomp-tests--foo 'foo 1)
                     (function-put 'bytecomp-tests--foo 'bar 2)
                     (defmacro bytecomp-tests--foobar ()
@@ -1636,7 +1665,8 @@ bytecomp-tests--not-writable-directory
            (byte-compile-error-on-warn t))
       (unwind-protect
           (progn
-            (write-region "" nil input-file nil nil nil 'excl)
+            (write-region ";;; -*-lexical-binding:t-*-\n"
+                          nil input-file nil nil nil 'excl)
             (write-region "" nil output-file nil nil nil 'excl)
             (set-file-modes input-file #o400)
             (set-file-modes output-file #o200)
@@ -1700,7 +1730,8 @@ bytecomp-tests--target-file-no-directory
     (let* ((default-directory directory)
            (byte-compile-dest-file-function (lambda (_) "test.elc"))
            (byte-compile-error-on-warn t))
-      (write-region "" nil "test.el" nil nil nil 'excl)
+      (write-region  ";;; -*-lexical-binding:t-*-\n"
+                     nil "test.el" nil nil nil 'excl)
       (should (byte-compile-file "test.el"))
       (should (file-regular-p "test.elc"))
       (should (cl-plusp (file-attribute-size
-- 
2.32.0 (Apple Git-132)

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Thu, 19 Oct 2023 13:48:21 +0200
> 
> The warning about a missing lexical-binding cookie rather belongs in the compiler than checkdoc, because it's not about documentation or style but code generation and ability to detect errors, both which are hindered by a missing cookie.
> 
> Moving the warning to the compiler also makes it more widely seen.

Adding Stefan, in case he has an opinion and/or comments.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The warning about a missing lexical-binding cookie rather belongs in the
>> compiler than checkdoc, because it's not about documentation or style but
>> code generation and ability to detect errors, both which are hindered by
>> a missing cookie.
>>
>> Moving the warning to the compiler also makes it more widely seen.

I think it's a great idea.  I hadn't noticed this warning in checkdoc,
else I would have probably suggested the same.

The patch looks good too (and the resulting simplicity, to me, is a good
hint that it's a better place to put this warning).


        Stefan

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Kangas]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> The warning about a missing lexical-binding cookie rather belongs in
> the compiler than checkdoc, because it's not about documentation or
> style but code generation and ability to detect errors, both which are
> hindered by a missing cookie.
>
> Moving the warning to the compiler also makes it more widely seen.

Yes, that makes much more sense.  Thanks for doing that.

The patch looks good too.

> +    ;;; My little pony mode  -*- lexical-binding: t -*-

If we want this to conform with the format required by package.el, this
should be:

;;; pony.el --- My little pony mode  -*- lexical-binding: t -*-

Or would that distract from the main point?

> +Lexical binding avoids some name conflicts and allows the compiler
> +to detect more mistakes and generate more efficient code.

On a side note, it would be good to also highlight this in the relevant
sections of the manual.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

19 okt. 2023 kl. 19.40 skrev Stefan Kangas <stefankangas@gmail.com>:

>> +    ;;; My little pony mode  -*- lexical-binding: t -*-
> 
> If we want this to conform with the format required by package.el, this
> should be:
> 
> ;;; pony.el --- My little pony mode  -*- lexical-binding: t -*-
> 
> Or would that distract from the main point?

Indeed the example was chosen to avoid anything irrelevant. If anything should go it would be the pony, but I put it there to remind the user that it's fine to include something else that obviously isn't parsed.
Something too formal-looking might give the impression of compulsory and/or machine-readable structure.

(I always found this traditional first line to be somewhat of an uneasy composite: too often it forces unnatural brevity on the description, or becomes so long that it wraps. I'd favour dedicating the first line to the -*- cookie alone.)

> On a side note, it would be good to also highlight this in the relevant
> sections of the manual.

Yes, the manual needs updating in some respects. I'll do this separately to keep the text as coherent as possible.

Thank you for the kind review!

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Kangas]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> (I always found this traditional first line to be somewhat of an
> uneasy composite: too often it forces unnatural brevity on the
> description, or becomes so long that it wraps. I'd favour dedicating
> the first line to the -*- cookie alone.)

I mostly agree, but removing the traditional file name is another
reasonable way to make the line a bit less unwieldy.  I'd be even more
inclined to make a change like that, myself.

Going on a tangent here, but I always assumed that the file name on the
first line was there for hysterical raisins, going back to the good old
days when people had to resort to sharing ELisp hacks on Usenet.  These
days, with GNU ELPA and git repositories, there is no risk for any
confusion about the name of a file.

I might be wrong though.  Perhaps it was always redundant?

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> The warning about a missing lexical-binding cookie rather belongs in
> the compiler than checkdoc, because it's not about documentation or
> style but code generation and ability to detect errors, both which are
> hindered by a missing cookie.
>
> Moving the warning to the compiler also makes it more widely seen.

So long as this warning is only displayed within code part of Emacs
itself, there are no valid objections to such a change.

But you have instead elected to generate warnings whenever such files
are byte-compiled.  There exist many packages which do not enable
lexical binding, whose authors have studiously elected not to: most of
Drew Adams' for example.  So this is tantamount to punitive action
against their users, in the form of an unsightly warning each time such
packages are installed.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> Cc: 66636@debbugs.gnu.org
> Date: Fri, 20 Oct 2023 14:09:45 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
> 
> > The warning about a missing lexical-binding cookie rather belongs in
> > the compiler than checkdoc, because it's not about documentation or
> > style but code generation and ability to detect errors, both which are
> > hindered by a missing cookie.
> >
> > Moving the warning to the compiler also makes it more widely seen.
> 
> So long as this warning is only displayed within code part of Emacs
> itself, there are no valid objections to such a change.
> 
> But you have instead elected to generate warnings whenever such files
> are byte-compiled.  There exist many packages which do not enable
> lexical binding, whose authors have studiously elected not to: most of
> Drew Adams' for example.  So this is tantamount to punitive action
> against their users, in the form of an unsightly warning each time such
> packages are installed.

Cannot such packages disable this warning in file-local variables?

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 66636@debbugs.gnu.org
>> Date: Fri, 20 Oct 2023 14:09:45 +0800
>> From:  Po Lu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
>> 
>> > The warning about a missing lexical-binding cookie rather belongs in
>> > the compiler than checkdoc, because it's not about documentation or
>> > style but code generation and ability to detect errors, both which are
>> > hindered by a missing cookie.
>> >
>> > Moving the warning to the compiler also makes it more widely seen.
>> 
>> So long as this warning is only displayed within code part of Emacs
>> itself, there are no valid objections to such a change.
>> 
>> But you have instead elected to generate warnings whenever such files
>> are byte-compiled.  There exist many packages which do not enable
>> lexical binding, whose authors have studiously elected not to: most of
>> Drew Adams' for example.  So this is tantamount to punitive action
>> against their users, in the form of an unsightly warning each time such
>> packages are installed.
>
> Cannot such packages disable this warning in file-local variables?

I don't think so, at least insomuch as byte-compile-warnings doesn't
function as a file-local variable.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  mattias.engdegard@gmail.com,
>   66636@debbugs.gnu.org
> Date: Fri, 20 Oct 2023 15:13:15 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Cc: 66636@debbugs.gnu.org
> >> Date: Fri, 20 Oct 2023 14:09:45 +0800
> >> From:  Po Lu via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> 
> >> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
> >> 
> >> > The warning about a missing lexical-binding cookie rather belongs in
> >> > the compiler than checkdoc, because it's not about documentation or
> >> > style but code generation and ability to detect errors, both which are
> >> > hindered by a missing cookie.
> >> >
> >> > Moving the warning to the compiler also makes it more widely seen.
> >> 
> >> So long as this warning is only displayed within code part of Emacs
> >> itself, there are no valid objections to such a change.
> >> 
> >> But you have instead elected to generate warnings whenever such files
> >> are byte-compiled.  There exist many packages which do not enable
> >> lexical binding, whose authors have studiously elected not to: most of
> >> Drew Adams' for example.  So this is tantamount to punitive action
> >> against their users, in the form of an unsightly warning each time such
> >> packages are installed.
> >
> > Cannot such packages disable this warning in file-local variables?
> 
> I don't think so, at least insomuch as byte-compile-warnings doesn't
> function as a file-local variable.

Then maybe we should allow that for this new warning (and potentially
also other warnings).

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> I don't think so, at least insomuch as byte-compile-warnings doesn't
>> function as a file-local variable.
>
> Then maybe we should allow that for this new warning (and potentially
> also other warnings).

If adding file local variables is an acceptable work-around, then adding
this to the .el file should be acceptable too:

    ;;; -*- lexical-binding:nil -*-

And that already works with the proposed patch.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> The warning about a missing lexical-binding cookie rather belongs in
>> the compiler than checkdoc, because it's not about documentation or
>> style but code generation and ability to detect errors, both which are
>> hindered by a missing cookie.
>>
>> Moving the warning to the compiler also makes it more widely seen.
>
> So long as this warning is only displayed within code part of Emacs
> itself, there are no valid objections to such a change.
>
> But you have instead elected to generate warnings whenever such files
> are byte-compiled.  There exist many packages which do not enable
> lexical binding, whose authors have studiously elected not to: most of
> Drew Adams' for example.  So this is tantamount to punitive action
> against their users, in the form of an unsightly warning each time such
> packages are installed.

No, it's just asking those authors to make the result of their studious
work explicit with

    -*- lexical-binding: nil -*-


-- Stefan

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

20 okt. 2023 kl. 15.38 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> No, it's just asking those authors to make the result of their studious
> work explicit with
> 
>    -*- lexical-binding: nil -*-

Right, and our sympathies are not so much for people dead set on writing new dynbound code as for users saddled with dusty decks.

Looks like there are no serious objections so the changes will be pushed soon, probably some time tomorrow.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 20 Oct 2023 19:40:02 +0200
> Cc: Po Lu <luangruo@yahoo.com>,
>  66636@debbugs.gnu.org,
>  Stefan Kangas <stefankangas@gmail.com>,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Looks like there are no serious objections so the changes will be pushed soon, probably some time tomorrow.

Really?  Is that what you deduced from the messages posted about this
change?  Did you see what Po Lu and I said?  What is your response to
that?

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

20 okt. 2023 kl. 21.27 skrev Eli Zaretskii <eliz@gnu.org>:

> Did you see what Po Lu and I said?

Not sure what he was trying to say, but it looks like a misunderstanding.
I'll try to clarify matters here. Please tell me if you still have concerns.

We certainly care about users of legacy code. The warning is intended as a soft nudge to encourage users to convert their code to lexical binding, which is quite straightforward most of the time.

When it's not, or when the user simply doesn't have the time or expertise to perform the conversion, it's just a matter of inserting `-*- lexical-binding: nil -*-`. Doing so will both silence the warning and buy the user some reprieve. A lot of reprieve, in fact.

This should probably be pointed out in the NEWS entry as well.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Sat, 21 Oct 2023 11:53:58 +0200
> Cc: monnier@iro.umontreal.ca,
>  luangruo@yahoo.com,
>  66636@debbugs.gnu.org,
>  stefankangas@gmail.com
> 
> We certainly care about users of legacy code. The warning is intended as a soft nudge to encourage users to convert their code to lexical binding, which is quite straightforward most of the time.
> 
> When it's not, or when the user simply doesn't have the time or expertise to perform the conversion, it's just a matter of inserting `-*- lexical-binding: nil -*-`. Doing so will both silence the warning and buy the user some reprieve. A lot of reprieve, in fact.
> 
> This should probably be pointed out in the NEWS entry as well.

If specifying lexical-binding:nil in the first line is the solution
for those who want to keep dynamically-bound code, then yes, it should
be definitely in NEWS, and probably also in the ELisp manual.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Mattias Engdegård]

21 okt. 2023 kl. 13.17 skrev Eli Zaretskii <eliz@gnu.org>:

> If specifying lexical-binding:nil in the first line is the solution
> for those who want to keep dynamically-bound code, then yes, it should
> be definitely in NEWS, and probably also in the ELisp manual.

Agreed on both points. The patch has been pushed with that change to NEWS, and Elisp manual revisions are upcoming.  We are a bit behind on lexical-binding in the manual so it could do with some more work.

Thanks to everyone who commented!

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Drew Adams]

> If specifying lexical-binding:nil in the first line is the solution
> for those who want to keep dynamically-bound code, then yes, it should
> be definitely in NEWS, and probably also in the ELisp manual.

Yes, it should be documented, but not just as
a simple solution, because it's not.

FWIW: It's not a solution for code that uses
`lexical-let[*]'.  That's simply been removed
now from Emacs, unfortunately.  It's not as
simple as just setting `lexical-binding' to nil
and replacing occurrences of `lexical-binding'
with `let[*]'.

And it's not even simple to try to include the
macros for `lexical-let[*]' in your code as a
temporary stop-gap.  Ditching `lexical-let[*]'
can really break things.  Just saying...

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> If specifying lexical-binding:nil in the first line is the solution
> for those who want to keep dynamically-bound code, then yes, it should
> be definitely in NEWS, and probably also in the ELisp manual.

FWIW, I think this would be a disservice to them (and to ourselves).
In 99% of the cases it's just as easy to make the code work with
`lexical-binding:t`.

We do want to allow people to silence the warning with
`lexical-binding:nil`, but we don't want to encourage it.


        Stefan

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Mattias Engdegård <mattias.engdegard@gmail.com>,
>   luangruo@yahoo.com,
>   66636@debbugs.gnu.org,  stefankangas@gmail.com
> Date: Sat, 21 Oct 2023 11:44:55 -0400
> 
> > If specifying lexical-binding:nil in the first line is the solution
> > for those who want to keep dynamically-bound code, then yes, it should
> > be definitely in NEWS, and probably also in the ELisp manual.
> 
> FWIW, I think this would be a disservice to them (and to ourselves).

What will?

> We do want to allow people to silence the warning with
> `lexical-binding:nil`, but we don't want to encourage it.

NEWS is not about encouraging anything, it is about fire escape.  The
manual encourages (or discourages) certain practices, but NEWS is
about something else.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> FWIW, I think this would be a disservice to them (and to ourselves).
> In 99% of the cases it's just as easy to make the code work with
> `lexical-binding:t`.

I think we want to encourage whatever relieves users of the most work,
not what comports best with some abstract conception of desirability.

Since the number of package developers does not exceed the number of
users installing packages, which goes without saying.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> FWIW, I think this would be a disservice to them (and to ourselves).
>> In 99% of the cases it's just as easy to make the code work with
>> `lexical-binding:t`.
> I think we want to encourage whatever relieves users of the most work,
> not what comports best with some abstract conception of desirability.

For users it should make no difference, they don't need to do anything
(other than avert their eyes from the warning, maybe).


        Stefan

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Mattias Engdegård
>  <mattias.engdegard@gmail.com>,  66636@debbugs.gnu.org,
>   stefankangas@gmail.com
> Date: Sun, 22 Oct 2023 00:12:51 -0400
> 
> >> FWIW, I think this would be a disservice to them (and to ourselves).
> >> In 99% of the cases it's just as easy to make the code work with
> >> `lexical-binding:t`.
> > I think we want to encourage whatever relieves users of the most work,
> > not what comports best with some abstract conception of desirability.
> 
> For users it should make no difference, they don't need to do anything
> (other than avert their eyes from the warning, maybe).

You interpret "users" in what I wrote too narrowly, I think.

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> >> FWIW, I think this would be a disservice to them (and to ourselves).
>> >> In 99% of the cases it's just as easy to make the code work with
>> >> `lexical-binding:t`.
>> > I think we want to encourage whatever relieves users of the most work,
>> > not what comports best with some abstract conception of desirability.
>> 
>> For users it should make no difference, they don't need to do anything
>> (other than avert their eyes from the warning, maybe).
>
> You interpret "users" in what I wrote too narrowly, I think.

My reply is to Po Lu who seemed to use "users" in the sense of "non
developers" (indeed, this use surprised me as well).


        Stefan

bug#66636: Move lexical-binding warning from checkdoc to byte-compiler

[Drew Adams]

> There exist many packages which do not enable
> lexical binding, whose authors have studiously
> elected not to: most of Drew Adams' for example.

Only because they are old or intended to
support also old Emacs releases.

I'm on record from Day One as _favoring_
the approach of Common Lisp (and now
Elisp) of lexical binding by default and
dynamic binding for variable declared as
special.

> So this is tantamount to punitive action
> against their users, in the form of an
> unsightly warning each time such
> packages are installed.

I can't speak for anyone else, obviously,
but for my part such warnings don't
particularly bother me for my libraries.

On the other hand, it's not clear to me
why we would have warnings other than for
byte-compiling.  (I just haven't been
following this - there might be a good
reason; dunno.)

The one thing that does bother me about
the change to what I prefer (lexical by
default) is the outright _removal_ of
`lexical-let[*]'.  This is really a
bother.  Code can be complex, and for a
library that has several uses of this
macro it's not necessarily simple to
modify it to use only (lexical) `let[*]'.

IMHO, it should have been (still should
be) possible to keep support for
`lexical-let[*]' around for much longer
- maybe forever.  It should be enough
to tell users not to use it for new code,
and why.
___

In _general_, I'm not crazy about Emacs
removing things, no matter how old or
deprecated.  If their code path isn't
used it should be no bother, especially
if it's relatively separate from other
code paths.

Deprecation alone generally means (1)
the thing is still supported but (2)
it's not enhanced.  And it would be
tolerable if deprecation even meant no
bug fixes.

An overeager-beaver impetus to _remove_
features is generally misguided, IMO.
But yeah, I'm not maintaining Emacs, so
this is easy for me to say.  (Just one
opinion.)