bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Philip Kaludercic]

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

The main intention here is to avoid underlining the first line in the
*scratch* buffer when flymake is enabled:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Don't warn about lexical binding in lisp-interaction-mode --]
[-- Type: text/x-patch, Size: 1130 bytes --]

From 714cba0f576566e39b7ffeb77b8f9b80bb3003ff Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Fri, 29 Mar 2024 12:30:59 +0100
Subject: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

* lisp/emacs-lisp/bytecomp.el (byte-compile-file): Suppress
warning in 'lisp-interaction-mode' buffers like the *scratch*
buffer.
---
 lisp/emacs-lisp/bytecomp.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 7af568cfe34..e04df8f9ade 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2234,7 +2234,8 @@ byte-compile-file
         (setq buffer-read-only nil
               filename buffer-file-name))
       ;; Don't inherit lexical-binding from caller (bug#12938).
-      (unless (local-variable-p 'lexical-binding)
+      (unless (or (local-variable-p 'lexical-binding)
+                  (derived-mode-p 'lisp-interaction-mode))
         (let ((byte-compile-current-buffer (current-buffer)))
           (displaying-byte-compile-warnings
            (byte-compile-warn-x
-- 
2.44.0


[-- Attachment #3: Type: text/plain, Size: 37 bytes --]


-- 
	Philip Kaludercic on peregrine

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Fri, 29 Mar 2024 11:31:49 +0000
> 
> The main intention here is to avoid underlining the first line in the
> *scratch* buffer when flymake is enabled:

If this is the goal, why such a general change?  Wouldn't it be better
to special-case *scratch* in Flymake instead?  Making such
generalizations runs the risk of unintended consequences.

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Fri, 29 Mar 2024 11:31:49 +0000
>> 
>> The main intention here is to avoid underlining the first line in the
>> *scratch* buffer when flymake is enabled:
>
> If this is the goal, why such a general change?  Wouldn't it be better
> to special-case *scratch* in Flymake instead?  Making such
> generalizations runs the risk of unintended consequences.

My thinking was that one never needs to set the variable anyway, since
`lisp-interaction-mode' sets `lexical-binding' anyway.  Alternatively,
one could also modify the patch to check if `lexical-binding' is non-nil?

-- 
	Philip Kaludercic on peregrine

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 70068@debbugs.gnu.org
> Date: Sat, 30 Mar 2024 11:07:00 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Date: Fri, 29 Mar 2024 11:31:49 +0000
> >> 
> >> The main intention here is to avoid underlining the first line in the
> >> *scratch* buffer when flymake is enabled:
> >
> > If this is the goal, why such a general change?  Wouldn't it be better
> > to special-case *scratch* in Flymake instead?  Making such
> > generalizations runs the risk of unintended consequences.
> 
> My thinking was that one never needs to set the variable anyway, since
> `lisp-interaction-mode' sets `lexical-binding' anyway.  Alternatively,
> one could also modify the patch to check if `lexical-binding' is non-nil?

The latter possibility would be better, I think.  Stefan, WDYT?

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

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

>> My thinking was that one never needs to set the variable anyway, since
>> `lisp-interaction-mode' sets `lexical-binding' anyway.  Alternatively,
>> one could also modify the patch to check if `lexical-binding' is non-nil?
>
> The latter possibility would be better, I think.  Stefan, WDYT?

Testing `lexical-binding` sounds good: the purpose is to encourage people
to activate `lexical-binding`, rather than encourage them to add a magic cookie.


        Stefan

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Philip Kaludercic]

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

>>> My thinking was that one never needs to set the variable anyway, since
>>> `lisp-interaction-mode' sets `lexical-binding' anyway.  Alternatively,
>>> one could also modify the patch to check if `lexical-binding' is non-nil?
>>
>> The latter possibility would be better, I think.  Stefan, WDYT?
>
> Testing `lexical-binding` sounds good: the purpose is to encourage people
> to activate `lexical-binding`, rather than encourage them to add a magic cookie.

OK, I have modified by patch and pushed it to master (0197db7b).

>
>         Stefan
>

-- 
	Philip Kaludercic on peregrine

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

> -      (unless (local-variable-p 'lexical-binding)
> +      (unless (or (local-variable-p 'lexical-binding)
> +                  (null lexical-binding))

That logic doesn't make sense to me, and it clearly doesn't work so I reverted it.
(For some reason the test didn't catch it, but it's now been made more robust.)

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Philip Kaludercic]

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

>> -      (unless (local-variable-p 'lexical-binding)
>> +      (unless (or (local-variable-p 'lexical-binding)
>> +                  (null lexical-binding))
>
> That logic doesn't make sense to me, and it clearly doesn't work so I reverted it.
> (For some reason the test didn't catch it, but it's now been made more robust.)

You are right, I made a mistake; What I want to say is this:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 5cff86784f0..a1194f2dc70 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2234,7 +2234,8 @@ byte-compile-file
         (setq buffer-read-only nil
               filename buffer-file-name))
       ;; Don't inherit lexical-binding from caller (bug#12938).
-      (unless (local-variable-p 'lexical-binding)
+      (when (or (not (local-variable-p 'lexical-binding))
+                (null lexical-binding))
         (let ((byte-compile-current-buffer (current-buffer)))
           (displaying-byte-compile-warnings
            (byte-compile-warn-x
--8<---------------cut here---------------end--------------->8---

What makes me uncertain is that you say it makes "no sense", so I  want
to make sure I am not misunderstanding something completely.

-- 
	Philip Kaludercic on peregrine

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

3 apr. 2024 kl. 20.17 skrev Philip Kaludercic <philipk@posteo.net>:

> You are right, I made a mistake; What I want to say is this:

> -      (unless (local-variable-p 'lexical-binding)
> +      (when (or (not (local-variable-p 'lexical-binding))
> +                (null lexical-binding))

Are you sure that you didn't mean

    (unless (or (local-variable-p 'lexical-binding)
                lexical-binding)

a.k.a.

    (when (and (not (local-variable-p 'lexical-binding)
               (not lexical-binding))

which may or may not make more sense? But how would either of these work, given that flymake runs a byte-compilation in a spawned Emacs and, as far as I can tell, propagates neither the buffer mode nor the lexical-binding value to that process?

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

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

If we want to do something about this, here are some options.

1. Add -*-lexical-binding:t-*- to initial-scratch-message.

2. It will sort itself out eventually as lexical-binding becomes the default. Sit and wait (a few years).

3. Something like the attached hack.


[-- Attachment #2: lexbind-warning-supp.diff --]
[-- Type: application/octet-stream, Size: 2788 bytes --]

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 5cff86784f0..fb3278c08ab 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2149,6 +2149,8 @@ byte-write-target-file
                 (cons tempfile target-file))
         (rename-file tempfile target-file t)))))
 
+(defvar bytecomp--inhibit-lexical-cookie-warning nil)
+
 ;;;###autoload
 (defun byte-compile-file (filename &optional load)
   "Compile a file of Lisp code named FILENAME into a file of byte code.
@@ -2234,7 +2236,8 @@ byte-compile-file
         (setq buffer-read-only nil
               filename buffer-file-name))
       ;; Don't inherit lexical-binding from caller (bug#12938).
-      (unless (local-variable-p 'lexical-binding)
+      (unless (or (local-variable-p 'lexical-binding)
+                  bytecomp--inhibit-lexical-cookie-warning)
         (let ((byte-compile-current-buffer (current-buffer)))
           (displaying-byte-compile-warnings
            (byte-compile-warn-x
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 8a713bd19a2..6bba1199aec 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -2169,6 +2169,8 @@ elisp-flymake-byte-compile-load-path
                                   (dolist (path x t) (unless (stringp path)
                                                        (throw 'tag nil)))))))
 
+(defvar bytecomp--inhibit-lexical-cookie-warning)
+
 ;;;###autoload
 (defun elisp-flymake-byte-compile (report-fn &rest _args)
   "A Flymake backend for elisp byte compilation.
@@ -2184,7 +2186,14 @@ elisp-flymake-byte-compile
     (save-restriction
       (widen)
       (write-region (point-min) (point-max) temp-file nil 'nomessage))
-    (let* ((output-buffer (generate-new-buffer " *elisp-flymake-byte-compile*")))
+    (let* ((output-buffer (generate-new-buffer " *elisp-flymake-byte-compile*"))
+           ;; Hack: suppress warning about missing lexical cookie if the
+           ;; buffer has `lexical-binding' set, so that we don't
+           ;; complain about *scratch* buffers.
+           (warning-suppression-opt
+            (and lexical-binding
+                 '("--eval"
+                   "(setq bytecomp--inhibit-lexical-cookie-warning t)"))))
       (setq
        elisp-flymake--byte-compile-process
        (make-process
@@ -2196,6 +2205,7 @@ elisp-flymake-byte-compile
                    ;; "--eval" "(setq load-prefer-newer t)" ; for testing
                    ,@(mapcan (lambda (path) (list "-L" path))
                              elisp-flymake-byte-compile-load-path)
+                   ,@warning-suppression-opt
                    "-f" "elisp-flymake--batch-compile-for-flymake"
                    ,temp-file)
         :connection-type 'pipe

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Philip Kaludercic]

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

> If we want to do something about this, here are some options.
>
> 1. Add -*-lexical-binding:t-*- to initial-scratch-message.
>
> 2. It will sort itself out eventually as lexical-binding becomes the default. Sit and wait (a few years).
>
> 3. Something like the attached hack.

I like the third option.

-- 
	Philip Kaludercic on peregrine

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  Eli Zaretskii
>  <eliz@gnu.org>,  70068@debbugs.gnu.org
> Date: Mon, 08 Apr 2024 07:21:43 +0000
> 
> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
> 
> > If we want to do something about this, here are some options.
> >
> > 1. Add -*-lexical-binding:t-*- to initial-scratch-message.
> >
> > 2. It will sort itself out eventually as lexical-binding becomes the default. Sit and wait (a few years).
> >
> > 3. Something like the attached hack.
> 
> I like the third option.

Agreed.

Stefan?

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

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

>> > If we want to do something about this, here are some options.
>> >
>> > 1. Add -*-lexical-binding:t-*- to initial-scratch-message.
>> >
>> > 2. It will sort itself out eventually as lexical-binding becomes the
>> > default. Sit and wait (a few years).
>> >
>> > 3. Something like the attached hack.
>> 
>> I like the third option.
>
> Agreed.
>
> Stefan?

I vote against 1, and I'm fine with both 2 and 3.


        Stefan

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

8 apr. 2024 kl. 14.02 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

>>> I like the third option.
>> 
>> Agreed.
> 
> I vote against 1, and I'm fine with both 2 and 3.

Looks like option 3 won. Actually I implemented option 3' which is like 3 but checks the mode instead of lexical-binding, on the grounds that a user who damages his lexical cookie by mistake wants to be reminded of it as soon as possible.

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Tue, 9 Apr 2024 10:57:40 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Philip Kaludercic <philipk@posteo.net>,
>  70068@debbugs.gnu.org
> 
> 8 apr. 2024 kl. 14.02 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
> 
> >>> I like the third option.
> >> 
> >> Agreed.
> > 
> > I vote against 1, and I'm fine with both 2 and 3.
> 
> Looks like option 3 won. Actually I implemented option 3' which is like 3 but checks the mode instead of lexical-binding, on the grounds that a user who damages his lexical cookie by mistake wants to be reminded of it as soon as possible.

That's not what I see in the changes you installed on master.

But if you want to check the mode, against what would you compare it?

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

9 apr. 2024 kl. 11.51 skrev Eli Zaretskii <eliz@gnu.org>:

>> Looks like option 3 won. Actually I implemented option 3' which is like 3 but checks the mode instead of lexical-binding, on the grounds that a user who damages his lexical cookie by mistake wants to be reminded of it as soon as possible.
> 
> That's not what I see in the changes you installed on master.

Then I made a mistake, but I don't see where. The intent is to have flymake add the argument

  --eval (setq bytecomp--inhibit-lexical-cookie-warning t)

if (derived-mode-p 'lisp-interaction-mode) is true, and then make the warning conditional on that variable. Doesn't it work?

What I can see is an unrelated bug in flymake: if the first line of a buffer (of an .el file) is empty, then the compiler emits a lexical cookie warning but flymake doesn't show it. Stefan, is this because there is no text to attach the warning to?

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Eli Zaretskii]

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Tue, 9 Apr 2024 12:45:18 +0200
> Cc: monnier@iro.umontreal.ca,
>  philipk@posteo.net,
>  70068@debbugs.gnu.org
> 
> 9 apr. 2024 kl. 11.51 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> >> Looks like option 3 won. Actually I implemented option 3' which is like 3 but checks the mode instead of lexical-binding, on the grounds that a user who damages his lexical cookie by mistake wants to be reminded of it as soon as possible.
> > 
> > That's not what I see in the changes you installed on master.
> 
> Then I made a mistake, but I don't see where. The intent is to have flymake add the argument
> 
>   --eval (setq bytecomp--inhibit-lexical-cookie-warning t)
> 
> if (derived-mode-p 'lisp-interaction-mode) is true, and then make the warning conditional on that variable. Doesn't it work?

Ah, okay, then it's my misunderstanding: I saw the
(local-variable-p 'lexical-binding) test and thought you
intended to replace that with a mode test.  Apologies.

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

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

> What I can see is an unrelated bug in flymake: if the first line of a buffer
> (of an .el file) is empty, then the compiler emits a lexical cookie warning
> but flymake doesn't show it.  Stefan, is this because there is no text to
> attach the warning to?

Sorry, I'm not very familiar with flymake.


        Stefan

bug#70068: [PATCH] Don't warn about lexical binding in lisp-interaction-mode

[Mattias Engdegård]

9 apr. 2024 kl. 15.35 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Sorry, I'm not very familiar with flymake.

I'll open a new bug for it. Closing this one.