Hi Ihor, 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.
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. 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=? 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? In these instances I've tentatively used =:eval= for now. I'm attaching an updated patch. Thanks very much! Cheers, Dominic On Sat, Sep 27, 2025 at 8:18 AM Ihor Radchenko <[email protected]> wrote: > > dominic meiser <[email protected]> writes: > > > I encountered what appears to be an issue with noweb reference expansion > > during tangling. I'm not sure if this is a known limitation, expected > > behavior, or if I'm misunderstanding how it should work, but I've prepared > > some patches that seem to fix the issue. > > > > The Problem: > > When using `:noweb tangle`, nested noweb references don't get recursively > > expanded. For example: > > ... > > Expected: The tangled output should contain "// block C content" > > Actual: The tangled output contains the unexpanded reference "<<ref3>>" > > ... > > Before I submit this as a proper patch series, I wanted to check: > > - Is this the expected behavior, or is it indeed a bug? > > This does indeed look like a genuine bug. > > > - If it's a bug, does this approach seem reasonable? > > Yes. > > > - Are there any other considerations I should account for? > > Your approach look reasonable, but you also need to make sure that all > the callers of `org-babel-expand-noweb-references' are updated. > > Also, I do not see you in FSF copyright records. Would you be interested > to complete the copyright assignment, so that we can accept large > contributions from you? See > https://orgmode.org/worg/org-contribute.html#copyright > > > The patches are available on my branch: > > `fix-noweb-tangle-recursive-expansion` > > Hmm. Which git server? > > -- > 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>
From 4fbea9484adda2bf7b42fa6a446674fc31ba9eca Mon Sep 17 00:00:00 2001 From: Dominic Meiser <[email protected]> Date: Tue, 30 Sep 2025 07:33:10 -0600 Subject: [PATCH] ob-core: Fix noweb tangle recursive expansion * 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. * lisp/ob-tangle.el (org-babel-tangle-single-block): Pass :tangle context to org-babel-expand-noweb-references. * lisp/ob-exp.el (org-babel-exp-process-buffer): Pass :export context when expanding inline source blocks during export. (org-babel-exp-code): Pass :export context when expanding code blocks for export. (org-babel-exp-results): Pass :eval context when evaluating blocks during export to generate results. * lisp/ob-lob.el (org-babel-lob-ingest): Pass :eval context when ingesting named blocks into the Library of Babel. * testing/lisp/test-ob-tangle.el (ob-tangle/expand-headers-as-noweb-references): Pass :tangle context to match tangle expansion behavior. (ob-tangle/noweb-tangle-recursive-expansion): New test verifying that :noweb tangle recursively expands nested noweb references during tangling. (ob-tangle/noweb-tangle-vs-export-contexts): New test verifying that noweb expansion respects different contexts. * testing/lisp/test-ob.el (test-ob/noweb-expansion): Pass :eval context in all test invocations. Previously, noweb expansion during tangling would fail to recursively expand nested references in blocks marked with :noweb tangle. The expansion context was hardcoded to :eval during recursive calls, meaning nested blocks would only expand if marked with any of the modes applicable for :eval. The context parameter determines which :noweb header argument values trigger expansion: - :tangle context: "tangle", "yes", "no-export", "strip-export", etc. - :export context: "yes", "strip-tangle" - :eval context: "eval", "yes", "no-export", "strip-export", etc. Reported-by: Dominic Meiser <[email protected]> --- lisp/ob-core.el | 29 +++++++++---- lisp/ob-exp.el | 6 +-- lisp/ob-lob.el | 2 +- lisp/ob-tangle.el | 3 +- testing/lisp/test-ob-tangle.el | 77 +++++++++++++++++++++++++++++++++- testing/lisp/test-ob.el | 16 +++---- 6 files changed, 112 insertions(+), 21 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 1402827e4..a99e74076 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -769,7 +769,7 @@ The src block is defined by its INFO, as returned by (let ((coderef (nth 6 info)) (expand (if (org-babel-noweb-p (nth 2 info) :eval) - (org-babel-expand-noweb-references info) + (org-babel-expand-noweb-references :eval info) (nth 1 info)))) (if (not coderef) expand (replace-regexp-in-string @@ -1003,7 +1003,7 @@ arguments and pop open the results in a preview buffer." (symbol-name (car el2))))))) (body (setf (nth 1 info) (if (org-babel-noweb-p params :eval) - (org-babel-expand-noweb-references info) (nth 1 info)))) + (org-babel-expand-noweb-references :eval info) (nth 1 info)))) (expand-cmd (intern (concat "org-babel-expand-body:" lang))) (assignments-cmd (intern (concat "org-babel-variable-assignments:" lang))) @@ -1143,7 +1143,7 @@ session." (user-error "No src code block at point") (setf (nth 1 info) (if (org-babel-noweb-p params :eval) - (org-babel-expand-noweb-references info) + (org-babel-expand-noweb-references :eval info) (nth 1 info))))) (session (cdr (assq :session params))) (dir (cdr (assq :dir params))) @@ -1513,7 +1513,7 @@ CONTEXT specifies the context of evaluation. It can be `:eval', (lang (nth 0 info)) (params (nth 2 info)) (body (if (org-babel-noweb-p params context) - (org-babel-expand-noweb-references info) + (org-babel-expand-noweb-references context info) (nth 1 info))) (expand-cmd (intern (concat "org-babel-expand-body:" lang))) (assignments-cmd (intern (concat "org-babel-variable-assignments:" @@ -3137,15 +3137,29 @@ CONTEXT may be one of :tangle, :export or :eval." (defvar org-babel-expand-noweb-references--cache-buffer nil "Cons (BUFFER . MODIFIED-TICK) for cached noweb references. See `org-babel-expand-noweb-references--cache'.") -(defun org-babel-expand-noweb-references (&optional info parent-buffer) +(defun org-babel-expand-noweb-references (context &optional info parent-buffer) "Expand Noweb references in the body of the current source code block. +CONTEXT specifies the context of expansion and must be one of :tangle, +:export, or :eval. + When optional argument INFO is non-nil, use the block defined by INFO instead. The block is assumed to be located in PARENT-BUFFER or current buffer \(when PARENT-BUFFER is nil). +The context determines which noweb header arguments are honored when +recursively expanding nested references: +- :tangle context: expands blocks with :noweb tangle, :noweb yes, etc. +- :export context: expands blocks with :noweb export, :noweb yes, etc. +- :eval context: expands blocks with :noweb eval, :noweb yes, etc. + +This is important for recursive expansion: when a block with :noweb tangle +references another block that also contains noweb references, those nested +references should only be expanded if the referenced block's :noweb setting +permits expansion in the tangle context. + For example the following reference would be replaced with the body of the source-code block named `example-block'. @@ -3207,8 +3221,9 @@ block but are passed literally to the \"example-block\"." (expand-body (i) ;; Expand body of code represented by block info I. - `(let ((b (if (org-babel-noweb-p (nth 2 ,i) :eval) - (org-babel-expand-noweb-references ,i) + `(let ((b (if (org-babel-noweb-p (nth 2 ,i) context) + (org-babel-expand-noweb-references + context ,i parent-buffer) (nth 1 ,i)))) (if (not comment) b (let ((cs (org-babel-tangle-comment-links ,i))) diff --git a/lisp/ob-exp.el b/lisp/ob-exp.el index ab99541ab..b71cf0116 100644 --- a/lisp/ob-exp.el +++ b/lisp/ob-exp.el @@ -215,7 +215,7 @@ this template." (string= "yes" (cdr (assq :noweb params)))) (org-babel-expand-noweb-references - info org-babel-exp-reference-buffer) + :export info org-babel-exp-reference-buffer) (nth 1 info))) (goto-char begin) (let ((replacement @@ -423,7 +423,7 @@ replaced with its value." (org-babel-noweb-wrap) "" (nth 1 info)) (if (org-babel-noweb-p (nth 2 info) :export) (org-babel-expand-noweb-references - info org-babel-exp-reference-buffer) + :export info org-babel-exp-reference-buffer) (nth 1 info)))) (org-fill-template (if (eq type 'inline) @@ -462,7 +462,7 @@ inhibit insertion of results into the buffer." (let ((lang (nth 0 info)) (body (if (org-babel-noweb-p (nth 2 info) :eval) (org-babel-expand-noweb-references - info org-babel-exp-reference-buffer) + :eval info org-babel-exp-reference-buffer) (nth 1 info))) (info (copy-sequence info)) (org-babel-current-src-block-location (point-marker))) diff --git a/lisp/ob-lob.el b/lisp/ob-lob.el index 472f9eda1..a3dc5e293 100644 --- a/lisp/ob-lob.el +++ b/lisp/ob-lob.el @@ -61,7 +61,7 @@ should not be inherited from a source block.") (when source-name (setf (nth 1 info) (if (org-babel-noweb-p (nth 2 info) :eval) - (org-babel-expand-noweb-references info) + (org-babel-expand-noweb-references :eval info) (nth 1 info))) (let ((source (intern source-name))) (setq org-babel-library-of-babel diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el index 4c224743b..7f61c5176 100644 --- a/lisp/ob-tangle.el +++ b/lisp/ob-tangle.el @@ -581,7 +581,8 @@ non-nil, return the full association list to be used by (let ((body (if (org-babel-noweb-p params :tangle) (if (string= "strip-tangle" (cdr (assq :noweb (nth 2 info)))) (replace-regexp-in-string (org-babel-noweb-wrap) "" (nth 1 info)) - (org-babel-expand-noweb-references info)) + (org-babel-expand-noweb-references + :tangle info)) (nth 1 info)))) (with-temp-buffer (insert diff --git a/testing/lisp/test-ob-tangle.el b/testing/lisp/test-ob-tangle.el index cd6876370..8e73e490f 100644 --- a/testing/lisp/test-ob-tangle.el +++ b/testing/lisp/test-ob-tangle.el @@ -78,7 +78,7 @@ "Test that references to headers are expanded during noweb expansion." (org-test-at-id "2409e8ba-7b5f-4678-8888-e48aa02d8cb4" (org-babel-next-src-block 2) - (let ((expanded (org-babel-expand-noweb-references))) + (let ((expanded (org-babel-expand-noweb-references :tangle))) (should (string-match (regexp-quote "simple") expanded)) (should (string-match (regexp-quote "length 14") expanded))))) @@ -764,6 +764,81 @@ This is to ensure that we properly resolve the buffer name." ;; Clean up the tangled file with the filename from org-test-with-temp-text-in-file (delete-file tangle-filename))))) +(ert-deftest ob-tangle/noweb-tangle-recursive-expansion () + "Test that :noweb tangle recursively expands nested noweb references." + (let ((file (make-temp-file "org-tangle-"))) + (unwind-protect + (progn + (org-test-with-temp-text-in-file + (format " +#+begin_src c :tangle %s :noweb tangle +// some code +<<noweb-ref1>> +<<noweb-ref2>> +#+end_src + +#+begin_src c :noweb-ref noweb-ref1 +// code from source block A +#+end_src + +#+begin_src c :noweb-ref noweb-ref2 :noweb tangle +// code from source block B +<<noweb-ref3>> +#+end_src + +#+begin_src c :noweb-ref noweb-ref3 +// code from source block C +#+end_src +" file) + (let ((org-babel-noweb-error-all-langs nil) + (org-babel-noweb-error-langs nil)) + (org-babel-tangle))) + (let ((tangled-content (with-temp-buffer + (insert-file-contents file) + (buffer-string)))) + ;; The tangled output should contain the content from block C + ;; (not the unexpanded <<noweb-ref3>> reference) + (should (string-match-p "// code from source block C" tangled-content)) + ;; The tangled output should NOT contain the unexpanded reference + (should-not (string-match-p "<<noweb-ref3>>" tangled-content)))) + (delete-file file)))) + +(ert-deftest ob-tangle/noweb-tangle-vs-export-contexts () + "Test that noweb expansion respects different contexts during tangle vs export." + (let ((tangle-file (make-temp-file "org-tangle-"))) + (unwind-protect + (progn + (org-test-with-temp-text-in-file + (format " +#+begin_src c :tangle %s :noweb yes +// tangled code +<<tangle-only>> +<<no-export>> +#+end_src + +#+begin_src c :noweb-ref tangle-only :noweb tangle +// visible during tangle +#+end_src + +#+begin_src c :noweb-ref no-export :noweb no-export +// visible during eval but not export +#+end_src +" tangle-file) + ;; Test tangling + (let ((org-babel-noweb-error-all-langs nil) + (org-babel-noweb-error-langs nil)) + (org-babel-tangle))) + + ;; Check tangled content + (let ((tangled-content (with-temp-buffer + (insert-file-contents tangle-file) + (buffer-string)))) + ;; Should have tangle-only content + (should (string-match-p "// visible during tangle" tangled-content)) + ;; Should have no-export content since :noweb no-export allows tangle context + (should (string-match-p "// visible during eval but not export" tangled-content)))) + (delete-file tangle-file)))) + (provide 'test-ob-tangle) ;;; test-ob-tangle.el ends here diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index 870296acf..ede55c49f 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -829,7 +829,7 @@ x #+begin_src sh bar #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; Handle :noweb-sep. (should (string= @@ -845,7 +845,7 @@ x #+begin_src sh :noweb-ref foo :noweb-sep \"\" baz #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; :noweb-ref is extracted from definition, not point of call. (should (string= @@ -869,7 +869,7 @@ x #+begin_src sh :noweb-sep \"\" (+ 1 1) #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; Handle recursive expansion. (should (equal "baz" @@ -887,7 +887,7 @@ x #+begin_src emacs-lisp baz #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; During recursive expansion, obey to `:noweb' property. (should (equal "<<bar>>" @@ -905,7 +905,7 @@ x #+begin_src emacs-lisp baz #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; Respect COMMENT headlines (should (equal "C" @@ -929,7 +929,7 @@ x #+begin_src emacs-lisp :noweb-ref foo C #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; Preserve case when replacing Noweb reference. (should (equal "(ignore)" @@ -941,7 +941,7 @@ x #+begin_src emacs-lisp :noweb yes<point> <<AA>> #+end_src" - (org-babel-expand-noweb-references)))) + (org-babel-expand-noweb-references :eval)))) ;; Test :noweb-ref expansion. (should (equal "(message \"!! %s\" \"Running confpkg-test-setup\") @@ -986,7 +986,7 @@ x " (goto-char (point-min)) (search-forward "begin_src") - (org-babel-expand-noweb-references))))) + (org-babel-expand-noweb-references :eval))))) (ert-deftest test-ob/splitting-variable-lists-in-references () (org-test-with-temp-text "" -- 2.51.0
