emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Nicolas Goaziou
Subject: Re: [O] [PATCH] Make lexical eval default for elisp src blocks
Date: Mon, 18 Apr 2016 18:38:43 +0200

Hello,

John Kitchin <address@hidden> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]