Re: [PATCH] Re: New source block results option for attaching file to node

[Ihor Radchenko <yantar92@gmail.com>]

Ryan Scott <ryan@vicarious-living.com> writes:

> Had no experience with the :DIR: property or writing unit tests for Org,
> but I think I've got both covered now.
> The ID creation prompting now only happens if there is no result from
> org-attach-dir, which should address the :DIR: case.

Thanks!

> Let me know if there's anything about those tests that I should modify.

Yeah. They do not pass, currently...
You can try yourself by running make test from Org git directory.

>> > +          ((or '(:dir . attach) '(:dir . "'attach"))
>> > +           (unless (org-id-get)
>> > +             (if (or noninteractive (y-or-n-p (format "Create ID for
>> entry \"%s\"?"
>> > +                                                      (org-get-heading
>> t t t t))))
>> > +                 (org-id-get-create)
>> > +               (error "Can't attach to entry \"%s\". Entry has no ID"
>> > +                      (org-get-heading t t t t))))

Unconditional ID creation for noninteractive is a bad idea. It is safer
to throw an error.

This code also generates warnings:


In end of data:
ob-core.el:2788:21: Warning: the function ‘org-id-get-create’ is not known to
    be defined.
ob-core.el:2787:58: Warning: the function ‘org-get-heading’ is not known to be
    defined.
ob-core.el:2785:23: Warning: the function ‘org-id-get’ is not known to be
    defined.
ob-core.el:2792:38: Warning: the function ‘org-attach-dir’ is not known to be
    defined.

Note that not all the ideas use org-id. Hence, `org-id-get-create' may
not be available during runtime. Adding (require 'org-id) is not a good
idea either (there will be other side-effects). So, if org-id is not
loaded, it will be better to just throw an error.

> +(ert-deftest test-ob-core/dir-attach ()
> +  "Test :dir header using special 'attach value"
> +  (should
> +   (org-test-with-temp-text-in-file
> +    "* Symbol
> +<point>#+begin_src elisp :dir 'attach :results file
> +(f-write-text \"attachment testing\" 'utf-8 \"test.txt\")

f-write-text requires f.el, which is external packages. You cannot use
it.

Also, while you are here, you can as well add tests for other possible
:dir settings.

Best,
Ihor