[BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[No Wayman]

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


I ran into this with some code I'm writing which checks against 
`lexical-binding'.
Should the following result in "lexical binding enabled" or 
"lexical binding disabled"?:

#+begin_src emacs-lisp :lexical t
(message "lexical binding %sabled" (if lexical-binding "en" 
"dis"))
#+end_src

Currently the `lexical-binding' variable is not bound because 
`org-babel-execute:emacs-lisp'
passes t to `eval', which enables lexical binding, but does not 
bind the `lexical-binding' variable.
The attached patch binds the variable in the lexical environment.
It's a matter of whether or not this is the right thing to do.
Thoughts?


Emacs  : GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X 
toolkit, cairo version 1.17.4, Xaw3d scroll bars)
 of 2021-09-04
Package: Org mode version 9.4.6 (9.4.6-ga451f9 @ 
/home/n/.emacs.d/straight/build/org/)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-babel-execute-bind-lexical-binding-var --]
[-- Type: text/x-patch, Size: 1520 bytes --]

From 42b92303faa86b1e9bd0692fca9469eb6a8f02ce Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
Date: Thu, 9 Sep 2021 16:40:02 -0400
Subject: [PATCH] ob-emacs-lisp: bind lexical-binding for :lexical t src blocks

* lisp/ob-emacs-lisp.el (org-babel-execute:emacs-lisp): bind lexical-binding in lexical environments

Because we use `eval' to execute the src block, this variable needs to
be explicitly set. Passing t as eval's LEXICAL arg will not bind
this variable.
---
 lisp/ob-emacs-lisp.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/ob-emacs-lisp.el b/lisp/ob-emacs-lisp.el
index d03151f13..6cf387b96 100644
--- a/lisp/ob-emacs-lisp.el
+++ b/lisp/ob-emacs-lisp.el
@@ -61,7 +61,7 @@ by `org-edit-src-code'.")
 
 (defun org-babel-execute:emacs-lisp (body params)
   "Execute a block of emacs-lisp code with Babel."
-  (let* ((lexical (cdr (assq :lexical params)))
+  (let* ((lexical (org-babel-emacs-lisp-lexical (cdr (assq :lexical params))))
 	 (result-params (cdr (assq :result-params params)))
 	 (body (format (if (member "output" result-params)
 			   "(with-output-to-string %s\n)"
@@ -71,7 +71,7 @@ by `org-edit-src-code'.")
 				     (member "pp" result-params))
 				 (concat "(pp " body ")")
 			       body))
-		       (org-babel-emacs-lisp-lexical lexical))))
+		       (when lexical (list (cons 'lexical-binding t) t)))))
     (org-babel-result-cond result-params
       (let ((print-level nil)
             (print-length nil))
-- 
2.33.0

Re: [BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[Tim Cross]

No Wayman <iarchivedmywholelife@gmail.com> writes:

> I ran into this with some code I'm writing which checks against 
> `lexical-binding'.
> Should the following result in "lexical binding enabled" or 
> "lexical binding disabled"?:
>
> #+begin_src emacs-lisp :lexical t
> (message "lexical binding %sabled" (if lexical-binding "en" 
> "dis"))
> #+end_src
>
> Currently the `lexical-binding' variable is not bound because 
> `org-babel-execute:emacs-lisp'
> passes t to `eval', which enables lexical binding, but does not 
> bind the `lexical-binding' variable.
> The attached patch binds the variable in the lexical environment.
> It's a matter of whether or not this is the right thing to do.
> Thoughts?
>
>

My thoughts on this would be that if lexical-bindings is supposed to be
bound to t, it should be done by eval when it gets a non-nil value for
it's optional argument. If I execute (eval FORM t) in an emacs lisp
buffer, it looks like lexical-bind is not set either, so I don't think
it should be in org either. To set it means we could get the same code
behaving differently depending on whether the source block is 'tangled'
and then evaluated or evaluated within org, which doesn't feel right.

Might be worth asking on emacs-devel why (eval FORM t) does not set this variable?

Re: [BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[No Wayman]

> My thoughts on this would be that if lexical-bindings is 
> supposed to be
> bound to t, it should be done by eval when it gets a non-nil 
> value for
> it's optional argument. If I execute (eval FORM t) in an emacs 
> lisp
> buffer, it looks like lexical-bind is not set either, so I don't 
> think
> it should be in org either. To set it means we could get the 
> same code
> behaving differently depending on whether the source block is 
> 'tangled'
> and then evaluated or evaluated within org, which doesn't feel 
> right.
  

I agree that we should avoid inconsistency. However it is already 
present.
You can verify this with the following src block:

#+begin_src emacs-lisp :lexical t :tangle ./dynamic.el :results 
 verbatim
(symbol-value lexical-binding)
#+end_src

1. `org-babel-execute-src-block' with point in the src block => 
nil (lexical binding enabled)
2. `org-edit-special' with point in the src block, 
`eval-last-sexp' within *Org Src* buffer => t (lexical binding 
enabled)
3. Tangle to the block to "dynamic.el", `eval-last-sexp' within 
"dynamic.el" => nil (dynamic binding)

> Might be worth asking on emacs-devel why (eval FORM t) does not 
> set this variable?

I understand why it doesn't. Elisp can be interpreted independent 
of a buffer, and different buffers need to distinguish which scope 
to use. 
It's more a question of what do we gain by leaving it unbound 
while evaluating a src block?
A user can rebind `lexical-binding' without hurting anything. The 
only case I can think of is if they intend to use it as a free 
variable, it won't be free. I can't imagine a use case where that 
would be true while the user is also explicitly requesting lexical 
binding, though.
IMO, it should be set during evaluation (addressed by my patch).

An alternative solution would be to allow one to set the lexical 
environment as part of the header args. e.g.

#+begin_src emacs-ilsp :lexical ((lexical-binding . t) t)
(symbol-value lexical-binding)
#+end_src

That way we offer the full flexibility of `eval' to those who 
would use it.

A separate issue, but one worth considering, is whether or not 
tangling a `:lexical t` src block should automatically add the 
file-local variable line in the tangled file.

Prior confusion (its not just me, I swear):

https://emacs.stackexchange.com/questions/61754/how-can-i-enable-lexical-binding-for-elisp-code-in-org-mode#comment100554_61758

https://lists.gnu.org/archive/html/emacs-orgmode/2019-08/msg00247.html

Re: [BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[Timothy]

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

Hi NoWayman,

> I ran into this with some code I’m writing which checks against
> `lexical-binding’.
> Should the following result in “lexical binding enabled” or
> “lexical binding disabled”?:

Can you think of any examples where this results in different behaviour (without
explicitly checking `lexical-binding')?

All the best,
Timothy

Re: [BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[No Wayman]

Timothy <tecosaur@gmail.com> writes:

> Hi NoWayman,
>
>> I ran into this with some code I’m writing which checks against
>> `lexical-binding’.
>> Should the following result in “lexical binding enabled” or
>> “lexical binding disabled”?:
>
> Can you think of any examples where this results in different 
> behaviour (without
> explicitly checking `lexical-binding')?
>
> All the best,
> Timothy


The third case I outlined.
Tangling a :lexical t src block sults in a dynamically scoped file 
unless the user manually inserts the file-local variable line.

Re: [BUG?][PATCH] Should the `lexical-binding' variable be bound during src block with :lexical t? [9.4.6 (9.4.6-ga451f9 @ /home/n/.emacs.d/straight/build/org/)]

[No Wayman]

No Wayman <iarchivedmywholelife@gmail.com> writes:

> The third case I outlined.
> Tangling a :lexical t src block sults in a dynamically scoped 
> file unless the
> user manually inserts the file-local variable line.

*results

That's adjacent to the issue I originally raised, but we could do 
better in that case.