dominic meiser <[email protected]> writes: > Thanks very much for the feedback. I've reworked my changes to make > the context a required parameter. I've updated the callers of > org-babel-expand-noweb-references. In many cases it was obvious what > the correct context should be.
Thanks! > There were several instances though where the context isn't obvious to > me. > > 1. *org-babel--expand-body* is called from both > =org-babel-confirm-evaluate= and =org-babel-execute-src-block=. For > =org-babel-execute-src-block= we surely want =:eval= but is that > the right behavior for org-babel-confirm-evaluate, too? I'm > assuming this is used during an interaction with the user where > they are presented with the code to be evaluated. Yes, should be :eval. When org-babel-execute-src-block or org-babel-confirm-evaluate are called, they are both called for the purposes of code evaluation, not to generate src block body for the purposes of tangling/export. Maybe we can document this in the org-babel--expand-body docstring for a good measure. Note that :eval scope is still relevant during export/tangling. For example, during export, when we need to evaluate the src block to get its results, we want full expansion to evaluate the body. We only do not want the noweb expansion when generating the src block body displayed in the exported document. > 2. *org-babel-expand-src-block* looks like an interactive command with > unknown user intent. Do we need some mechanism to determine user > intent? Or is it ok to assume =:eval=? Yes, assume :eval (arbitrarily - the situation si ambiguous). Simply to follow the existing behavior. > 3. *org-babel-lob-ingest* I'm not familiar with lob > functionality. Are these blocks always ultimately intended for > evaluation? Or could they also be used for export? They can indeed be used for export. org-babel-lob-get-info uses the expanded body and is called by org-babel-exp-process-buffer. It looks like a bug. We should probably avoid expanding noweb references in org-babel-lob-ingest. The return value of org-babel-log-get-info is only passed to org-babel-execute-src-block or to org-babel-exp-do-export. Both these functions are handling noweb expansion anyway. > * lisp/ob-core.el (org-babel-expand-noweb-references): Add required > CONTEXT parameter specifying the expansion context (:tangle, :export, > or :eval). Use the provided context when recursively expanding nested > noweb references. Update documentation to explain the context > parameter and its role in recursive expansion. This will create a breaking change in a user-facing API function. I'd prefer to avoid such breaking changes. I suggest to add CONTEXT as optional parameter, after INFO and PARENT-BUFFER By default, if CONTEXT Is not passed, assume :eval. This will avoid breaking third-party code that is using org-babel-expand-noweb-references. > * lisp/ob-lob.el (org-babel-lob-ingest): Pass :eval context when > ingesting named blocks into the Library of Babel. We should probably drop calling noweb expansion altogether here. But we need to make sure that https://lists.gnu.org/archive/html/emacs-orgmode/2017-09/msg00361.html does not re-appear. -- Ihor Radchenko // yantar92, Org mode maintainer, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
