[PATCH v3] ob-tangle.el: Blocks overwrite each other when grouping before tangling

[Evgenii Klimov]

Hi

Here are the new tests that demonstrate the bug in block grouping during
block collection, along with the patch to address the issue, taking your
previous remarks into account.

I split it into two patches so you can apply the tests first to see the
bug.  And probably tests should be rewritten as they look too complex
and mostly duplicate each other.  I'd appreciate your suggestions
on how to enhance them.

Ihor Radchenko <yantar92@posteo.net> writes:

> Evgenii Klimov <eugene.dev@lipklim.org> writes:
>
>> In this version I just updated the docstrings for the relevant
>> functions, because prior to that it wasn't clear: does this "default
>> export file for *all* source blocks" influence blocks with :tangle
>> "yes"/FILENAME?
>
> Thanks for the patch, but we need to be careful changing things in
> ob-tangle. Not everything is well-documented there.
>
>>  Optional argument TARGET-FILE can be used to specify a default
>> -export file for all source blocks.
>> +export file for all source blocks without :tangle header
>> +argument.
>
> This is confusing.
> Is :tangle yes "without"?
> What about inheritance?
> What about default header args?

I just find current lack of details confusing as well and want to
express the place of TARGET-FILE in the lineage of :tangle in
~org-babel-get-src-block-info~:
    1. org-babel-default-header-args
       1. TANGLE-FILE of ~org-babel-tangle~
    2. org-babel-default-header-args:<lang>
    3. org-babel-params-from-properties
    4. org-element-property :parameters datum
    5. org-element-property :header datum

It wasn't clear for me: will ":tangle yes" or explicit ":tangle no" be
affected by TARGET-FILE.  Maybe if we rephrase as follows it will be
clear for both of us:

    Optional argument TARGET-FILE can be used to overwrite a default
    export file in `org-babel-default-header-args' for all source
    blocks.

> What if we have :tangle "/path/to/foo" and TARGET-FILE = "/path/to/foo"?
> What if they are :tangle "./foo" and TARGET-FILE = "/full/path/to/foo"?

See the new tests in the patch, I tried to take it into account.

>>  (defun org-babel-effective-tangled-filename (buffer-fn src-lang src-tfile)
>> -  "Return effective tangled filename of a source-code block.
>> +  "Return effective tangled absolute filename of a source-code block.
>
> This will likely cause breakage.
> There are two callers of `org-babel-effective-tangled-filename:
> 1. `org-babel-tangle-collect-blocks'
> 2. `org-babel-tangle-single-block'
>
> `org-babel-tangle-single-block' passes (nth 1 result) as BUFFER-FN.
> Its value is
>
> (if org-babel-tangle-use-relative-file-links
>                   (file-relative-name file)
>                 file)
>
> So,
>
>> +  (let* ((fnd (file-name-directory (buffer-file-name
>> +                                    (get-buffer buffer-fn))))
>
> will fail when FILE contains file path.
> And it does: (file (buffer-file-name (buffer-base-buffer)))

Thanks, fixed: both `org-babel-tangle-single-block' and
`org-babel-tangle-collect-blocks' now pass absolute value to
`org-babel-effective-tangled-filename'.

v3-0001-testing-lisp-test-ob-tangle.el-Test-block-collect.patch
Description: Text Data

<<< message/external-body; name="/home/eugene/git/org-mode/patches/v3-0002-ob-tangle.el-Avoid-relative-file-names-when-group.patch"; access-type=local-file: Unrecognized >>>