Re: [PATCH] ob-tangle.el: restore :tangle closure nil behavior

[Tom Gillespie <tgbugs@gmail.com>]

On Wed, Aug 16, 2023 at 2:09 AM Ihor Radchenko <yantar92@posteo.net> wrote:
>
> Tom Gillespie <tgbugs@gmail.com> writes:
>
> > Subject: [PATCH] ob-tangle.el: restore :tangle closure evaluation before eval
> >  info
> > This patch fixes a bug where header arguments like :tangle (or "no")
> > were treated as if they were tangling to a file named "(or \"no\")".
> > As a result, org-bable would call org-babel-get-src-block-info with
> > 'no-eval set to nil, causing parameters to be evaluated despite the
> > fact that when :tangle no or equivalent is set, the other parameters
> > should never be evaluated.
>
> What do you mean by "restore"? Were it evaluated in the past?
> May you please provide a reproducer?

Hrm. I think I may have mixed two commit lines. It is the case that
:tangle closures used to work, but you are right, the historical behavior
when tangling closures meant that all parameters were evaluated (tested
with the block below in 27 and 28).

#+begin_src elisp :var value=(error "oops") :tangle (or "no")
value
#+end_src

My use case is that I have blocks that I want to tangle that set :var
from e.g. the library of babel, which isn't always loaded, but which also
is not required if :tangle is no.

> > -(defun org-babel-tangle--unbracketed-link (params)
> > +(defun org-babel-tangle--unbracketed-link (params &optional info-was-evaled)
>
> This is not acceptable. Taking care about evaluating INFO should be done
> in a single place instead of adding checks across the babel code. If we
> go the proposed way, I expect a number of bugs appearing when somebody
> forgets to change the eval check in some place.

I don't like the solution either. I see two potential alternatives.
1. change the structure of the info list to indicate whether it has
already been evaluated
2. always call org-babel-read on (cdr (assq :tangle params)) even
if it may already have been evaluated which can lead to some unexpected
and potentially nasty results.

I don't think we can consolidate evaluating parameters
into one place in the general case because there are
order dependencies where a setting in one param header
should mask others (as is the case here). In principle we
could consolidate them, but I think that would add significant
complexity because we would have to push all the logic for
handling whether a given ordering restriction applies inside
that location. e.g. if I have a block set :eval (if ev "yes" "no")
it would be bad form to evaluate the parameters before determining
whether the :eval closure evaluates to "yes" or "no". Should that
go inside org-process-params, or should it be handled locally
by e.g. org-babel-tangle and org-babel-execute-src-block separately?

Thoughts?