On Wed, Aug 16, 2023 at 2:09 AM Ihor Radchenko <yanta...@posteo.net> wrote: > > Tom Gillespie <tgb...@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?
