Re: [PATCH] Make lexical eval default for elisp src blocks

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

John Kitchin <jkitchin@andrew.cmu.edu> writes:

> Set default in `org-babel-default-header-args:emacs-lisp'. Add an
> optional argument to the eval function.

Thanks for the patch. Some comments follow.

> +*** Default lexical evaluation of emacs-lisp src blocks
> +Emacs-lisp src blocks in babel are now evaluated using lexical scoping. There is a new header to control this behavior.
> +
> +The default results in an eval with lexical scoping.
> +:lexical yes
> +
> +This turns lexical scoping off in the eval (the former behavior).
> +:lexical no
> +
> +This uses the lexical environment with x=42 in the eval.
> +:lexical '((x . 42))

According to the Elisp manual:

     The value of LEXICAL can also be a non-empty alist specifying
     a particular "lexical environment" for lexical bindings; however,
     this feature is only useful for specialized purposes, such as in
     Emacs Lisp debuggers. *Note Lexical Binding::.

I'm not opposed to it, but is there any reason to include the
opportunity to specify an alist here?

> +(defvar org-babel-default-header-args:emacs-lisp
> +  '((:lexical . "yes"))
> +  "Default arguments for evaluating an emacs-lisp source block.
> +
> +:lexical is \"yes\" by default and causes src blocks to be eval'd
> +using lexical scoping. It can also be an alist mapping symbols to
> +their value. It is used as the optional LEXICAL argument to
> +`eval'.")

You need to separate sentences with two spaces.

You also need to add (defconst org-babel-header-args:emacs-lisp). See
for example `org-babel-header-args:R'.

>  (defun org-babel-expand-body:emacs-lisp (body params)
>    "Expand BODY according to PARAMS, return the expanded body."
> @@ -51,13 +57,18 @@
>  (defun org-babel-execute:emacs-lisp (body params)
>    "Execute a block of emacs-lisp code with Babel."
>    (save-window-excursion
> -    (let ((result
> -           (eval (read (format (if (member "output"
> -                                           (cdr (assoc :result-params params)))
> -                                   "(with-output-to-string %s)"
> -                                 "(progn %s)")
> -                               (org-babel-expand-body:emacs-lisp
> -                                body params))))))
> +    (let* ((lexical (cdr (assoc :lexical params)))

Nitpick: (cdr (assq :lexical params))

> +	   (result
> +	    (eval (read (format (if (member "output"
> +					    (cdr (assoc :result-params params)))

Nitpick, while you're at it: (cdr (assq :result-params params))

> +				    "(with-output-to-string %s)"
> +				  "(progn %s)")
> +				(org-babel-expand-body:emacs-lisp
> +				 body params)))
> +
> +		  (if (listp lexical)
> +		      lexical
> +		    (string= "yes" lexical)))))

There is no support for t. I think we should allow both :lexical
t and :lexical yes.


Regards,

-- 
Nicolas Goaziou