Hi Ihor, thanks for the review. > In org-babel-tangle-collect-blocks: > ob-tangle.el:494:10: Warning: Unused lexical variable ‘buffer-fn’ > In org-babel-tangle--concat-targets: > ob-tangle.el:572:6: Warning: value from call to ‘remove’ is unused Fixed, thanks for pointing those out.
> Please add a proper commit message. Done. Please check the updated commit message. > Won't this break when there are multiple :tangle targets? > For multiple targets, `org-babel-tangle-single-block' now returns a list > of lists so (car block) changed its meaning compared to pre-patch. Oops, you're right. That was a mistake on my part. I've corrected it now. > Please update the docstring to describe function arguments properly. Fixed. > `org-babel-get-src-block-info' should compute the parameter value. You're right. I’ve removed all the manual evals as you suggested. > I'd rather avoid such non-deterministic return value. > Having different return values depending on the :tangle parameter is, > IMHO, not the best design. I'd rather go with my idea of adding a new > optional argument that will turn on the new return value convention and > fall back to legacy by default. I agree with what you suggested. However, I’m currently struggling to implement it in a way that cleanly addresses a couple of issues: 1. When `only-this-block' is nil and :tangle has multiple targets, the link in the return result becomes ambiguous. 2. As I mentioned, I'm unsure how to handle the output of org-babel-tangle--unbracketed-link for its use in org-babel-tangle-comment-links. I'm open to suggestions. llcc On Sun, Apr 13, 2025 at 1:24 AM Ihor Radchenko <yanta...@posteo.net> wrote: > > Lei Zhe <lzhe...@gmail.com> writes: > > >> how can users of `org-babel-tangle-single-block' figure out which variant > >> of the return value is to expect? > > It returns the correct relative or absolute link that users can expect > > when :tangle is a single path, while it returns an absolute link when > > :tangle is a list. > > It’s a tradeoff, though, because `org-babel-tangle--unbracketed-link' > > is also used by `org-babel-tangle-comment-links', which adds > > complexity to handling it fully correctly. > > I'd rather avoid such non-deterministic return value. > Having different return values depending on the :tangle parameter is, > IMHO, not the best design. I'd rather go with my idea of adding a new > optional argument that will turn on the new return value convention and > fall back to legacy by default. > > > I’ve added more test cases, and all of them passed on macOS. > > Thanks! Tests are passing on my side as well. > > However, there are compiler warnings that indicate problems: > > In org-babel-tangle-collect-blocks: > ob-tangle.el:494:10: Warning: Unused lexical variable ‘buffer-fn’ > > In org-babel-tangle--concat-targets: > ob-tangle.el:572:6: Warning: value from call to ‘remove’ is unused > > > > Subject: [PATCH] New feature: tangle org source blocks to multiple targets > > > > 1. add `:tangle-directory' to specify tangle directory. > > 2. `:tangle' now accepts symbols that return a path string, or a list of > > file path, or a single string. > > Please add a proper commit message. See > https://orgmode.org/worg/org-contribute.html#commit-messages > > > + (let* ((block (org-babel-tangle-single-block counter t)) > > + (src-file (car block)) > > + (src-lang (caar block))) > > + (unless (or (not src-file) > > Won't this break when there are multiple :tangle targets? > For multiple targets, `org-babel-tangle-single-block' now returns a list > of lists so (car block) changed its meaning compared to pre-patch. > > > +(defun org-babel-tangle--concat-targets (buffer-fn info) > > + "Return a list of tangled files based on the `:tangle' > > +and `:tangle-directory' in PARAMS." > > Please update the docstring to describe function arguments properly. > > > + (let* ((params (nth 2 info)) > > + (src-lang (nth 0 info)) > > + (src-tdirectories (cdr (assq :tangle-directory params))) > > + (src-tfiles (cdr (assq :tangle params))) > > + (src-tfiles (pcase (type-of src-tfiles) > > + ('cons src-tfiles) > > + ('symbol (eval src-tfiles)) > > + (_ (eval src-tfiles))))) > > No eval please. `org-babel-get-src-block-info' should compute the > parameter value. We should generally avoid using `eval' except few > places to reduce risks of unsafe code evaluation. > > -- > 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>
0001-ob-tangle.el-Support-tangling-a-source-block-to-mult.patch
Description: Binary data
