Re: [PATCH] lisp/ob-core.el: pass expanded body to org-confirm-babel-evaluate

[Kyle Meyer <kyle@kyleam.com>]

Tom Gillespie writes:

> On Sun, Jul 19, 2020 at 2:13 PM Kyle Meyer <kyle@kyleam.com> wrote:

>> An option not mentioned above is to replace (nth 1 info) with the
>> expanded body upstream of (when (org-babel-check-evaluate info) ...).
>> Modifying the body in INFO is admittedly not pretty, but it's in line
>> with what is done elsewhere (e.g., -expand-src-block,
>> -exp-process-buffer, -load-in-session), as well as with how other INFO
>> elements in -execute-src-block are handled.
>
> I considered this as well, and in fact I assumed that this is how it worked
> before I looked to see the code was actually doing. I didn't know what the
> consequences of making it work that way would be and tend to err on the
> side of not kobbering data that some other function might expect to be in
> an unmodified state. This would certainly be the most expedient solution.
>
> There would need to be a way to indicate that info should not expand noweb
> references so that :noweb no-export can get the unexpanded form. Maybe
> and optional argument =expand= that defaults to t?
> (defun org-babel-get-src-block-info (&optional light datum expand)
> ... )

Hmm, it looks like you're thinking of a different spot than I was
referring to.  I was talking about modifying (nth 1 info) in
-execute-src-block, before the (when (org-babel-check-evaluate ...).

Prior to commit 3b3fc520a, (nth 1 info) was modified in
-execute-src-block but after org-babel-check-evaluate and
org-babel-confirm-evaluate.  That makes me think it'd probably be safe
to do again, just a bit upstream in the same function.  And I don't see
a spot where modifying that element would be problematic.  On the
incoming end, INFO is passed to copy-tree if it is provided as an
argument, and I don't think any of the functions that -execute-src-block
feeds INFO to would be affected.  But I of course could be overlooking
something.

On the other hand, if org-babel-get-src-block-info did the expansion,
I'd be a bit more worried about downstream effects (though I haven't
tried to trace things too closely).  Also, -execute-src-block takes a
PARAMS argument, and I think that needs to be merged with (nth 2 info)
before considering whether to expand, so it seems like expanding in
-get-src-block-info would be too soon.

Regardless of whether modifying (nth 1 info) would work, I also think
it'd be fine to instead go ahead with something along the lines of your
initial patch.  In that case, the main question is whether coderefs
should be removed (as they currently are in your patch).  If not,
perhaps something like below (untested) would suffice.

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index e798595bd..230706b7f 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -238,7 +238,10 @@ (defun org-babel-check-confirm-evaluate (info)
 		    (if (functionp org-confirm-babel-evaluate)
 			(funcall org-confirm-babel-evaluate
 				 ;; Language, code block body.
-				 (nth 0 info) (nth 1 info))
+				 (nth 0 info)
+				 (if (org-babel-noweb-p headers :eval)
+				     (org-babel-expand-noweb-references info)
+				   (nth 1 info)))
 		      org-confirm-babel-evaluate))))
     (cond
      (noeval nil)