Tom Gillespie writes: > This is a patch to improve the behavior of > org-babel-check-confirm-evaluate and the usefulness of > org-confirm-babel-evaluate when a function is provided.
Thank you. > This commit changes the behavior of org-babel-check-confirm-evaluate > so that org-confirm-babel-evaluate receives the fully expanded code to > be evaluated. This is important because there is no easy way to expand > noweb references inside org-confirm-babel-evaluate without calling > org-babel-get-src-block-info a second time. It is also important > because the current behavior makes it possible for code lurking behind > noweb references to change without the user being able to detect those > changes if they trust org-confirm-babel-evaluate is receiving the > actual body of the code that will be evalulated. I find that convincing. I'd guess that a org-confirm-babel-evaluate function would always want to see expanded noweb references. This doesn't mention coderefs, though, and the case there seems less clear. If there's not a strong reason to strip coderefs, then the new function wouldn't be necessary, and -check-confirm-evaluate could instead go with the pattern used elsewhere: (if (org-babel-noweb-p (nth 2 info) :eval) (org-babel-expand-noweb-references info) (nth 1 info)) (Perhaps it'd be worth adding a -maybe-expand variant of -expand-noweb-references since there are a good number of spots that do the same params check before calling -expand-noweb-references.) > These changes were made in such a way as to minimize changes to the > existing functions, however they come at the cost of making two calls > to org-babel-get-body-to-eval [...] Or potentially three calls to -get-body-to-eval: one call with the -check-evaluate call in -execute-src-block, one with the -confirm-evaluate in -execute-src-block, and then the direct call to -get-body-to-eval in -execute-src-block. > [...] since passing the expanded body through to > org-confirm-babel-evaluate would either require appending it to the > info list or changing the function signatures of > org-babel-confirm-evaluate and org-babel-check-confirm-evaluate which > is undesireable. Furthermore, to compute the expanded body only once > would require switching from using cond in org-babel-execute-src-block > to using a series of nested ifs. This change can be made, but starting > from the current implementation will provide a working reference for > comparison rather than trying to make all the changes at the same > time. 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. In fact, -execute-src-block did modify (nth 1 info) before 3b3fc520a (Fix coderef handling in source blocks, 2016-08-28), though too late to affect calls to a org-confirm-babel-evaluate function. Quickly checking the tests added in that commit as well as the example in the message [0] that prompted that commit, modifying (nth 1 info) didn't seem to break the fix there. [0] https://orgmode.org/list/87mvjyoja2.fsf@dell-desktop.WORKGROUP