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:

> Here's my latest patch.
> Uses special :dir value 'attach to use attachment directory as working dir.
> Now prompts to create IDs for nodes that are missing.
> Solved a handful of issues with my previous versions of this and I've been
> using it regularly for a bit now.
>
> I've added documentation and completed the copyright assignment to the FSF.

I have consulted Bastien about our merge policy and apparently it is ok
to merge staff into files without maintainer. ob-core.el is one of such
files.

So, I did a more elaborate testing of your patch. I have some comments.

Firstly, the patch does not apply onto current main.

> +Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
> ...
> +Passing the symbol ~attach~ or string ='attach= to the =:dir= option

When I was trying to use your patch, I felt slightly confused about the
"'attach" part. It _looks_ like a type and I first tried to do :dir
"attach" yielding predictable lack of attachment: file link.

Maybe you can say "or the string ~"'attach"~ (with quote)".

> +                 (org-babel-result-to-file
> +		  result
> +		  (org-babel--file-desc (nth 2 info) result)
> +                  'attachment))))

There are only 2 calls to org-babel-result-to-file in the whole Org
codebase. And you did not change the second call, which yield strange
side-effects:

#+NAME: attr_wrap
#+BEGIN_SRC sh :var data="" :var width="\\textwidth" :results output
  echo "#+ATTR_LATEX: :width $width"
  echo "$data"
#+END_SRC

#+begin_src gnuplot :dir 'attach :file test.png
plot x
#+end_src

#+RESULTS:
[[attachment:test.png]]

#+begin_src gnuplot :dir 'attach :file test.png  :post attr_wrap(width="5cm", data=*this*) :results drawer
plot x
#+end_src

#+RESULTS:
:results:
#+ATTR_LATEX: :width 5cm
[[file:data/4d/6a76f8-4016-4edf-9d26-e0b3a634dbc1/test20.png]] <----!!!!! this is not expected
:end:

> +If the optional TYPE is passed as 'attachment` and the path is a
> +descendant of the DEFAULT-DIRECTORY, the generated link will be
> +specified as an an \"attachment:\" style link"
> +           (in-attach-dir (when (and request-attachment (> (length result-file-name) attach-dir-len))
> +                            (string=
> +                             (substring result-file-name 0 attach-dir-len)
> +                             attach-dir))))

This is a risky heuristics.
One can do something like
(setq org-attach-id-dir "/tmp/alsjkdsalkjdlaskdjklasjdlkasjdlkasjdlkajdklasjdlasjlasdjk/")
and often get your heuristics fail.
Of course, it will require some terribly noncomplying ob-* library that
will create file disregarding default-directory, but still...

> +             (if (y-or-n-p (format "Create ID for entry \"%s\"?"
> +                                   (org-get-heading t t t t)))

This is a nice dialogue, but note that Emacs can run noninteractively or
execute source block during export (including asynchronous). I would
guard the query after consulting `noninteractive' variable.


Best,
Ihor