New source block results option for attaching file to node

New source block results option for attaching file to node

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 239 bytes --]

An additional option for use with ":results file" that moves the returned
path to the node attachment directory (as returned by org-attach-dir),
creating it if necessary.

First time submitting a patch. Any feedback is appreciated.

-ryan

[-- Attachment #1.2: Type: text/html, Size: 338 bytes --]

[-- Attachment #2: 0001-ob-core-Added-option-for-attaching-file-from-babel-s.patch --]
[-- Type: application/octet-stream, Size: 4923 bytes --]

From cbe97587df4b660f56e420015222e4f3559588a2 Mon Sep 17 00:00:00 2001
From: "Ryan C. Scott" <ryan@5pmcasual.com>
Date: Wed, 25 Aug 2021 02:15:09 -0700
Subject: [PATCH] ob-core: Added option for attaching file from babel src block
 result

* ob-core.el (org-babel-format-result): Added `:results-param' option `attach' which when used with `file' will cause the resultant file path to be moved to the `org-attach-dir' for the node.
(org-babel-result-to-file): Added optional 3rd parameter `type' which will be used in place of "file" in link type, if present.
(org-babel-result-to-file-attachment): New function that moves the file at theh provided `result' path to the nodes attachment directory, creating it as necessary using `org-attach-dir'.  A new function was created to adhere to the design of the API and not complicated `org-babel-result-to-file' with the movement to the attachment directory.

* org-attach.el (org-attach-dir): Added autoload header to simplify dependencies necessary to support this feature (called in `org-babel-result-to-file-attachment').
---
 lisp/ob-core.el    | 31 ++++++++++++++++++++++++++-----
 lisp/org-attach.el |  1 +
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 857e03e55..c1d1b77e0 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2188,6 +2188,10 @@ silent -- no results are inserted into the Org buffer but
 file ---- the results are interpreted as a file path, and are
           inserted into the buffer using the Org file syntax.
 
+attach -- used with `file', will move the file at the path returned
+          by the source block to the nodes attachmen directory (as
+          reported by `org-attach-dir'.
+
 list ---- the results are interpreted as an Org list.
 
 raw ----- results are added directly to the Org file.  This is
@@ -2241,9 +2245,13 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result (if (member "attach" result-params)
+                            (org-babel-result-to-file-attachment
+                             result
+                             (org-babel--file-desc (nth 2 info) result))
+                          (org-babel-result-to-file
+			   result
+			   (org-babel--file-desc (nth 2 info) result))))))
 	((listp result))
 	(t (setq result (format "%S" result))))
   (if (and result-params (member "silent" result-params))
@@ -2548,7 +2556,7 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
 file's directory then expand relative links."
@@ -2559,7 +2567,8 @@ file's directory then expand relative links."
 			    (expand-file-name
 			     (file-name-directory
 			      (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
+      (format "[[%s:%s]%s]"
+              (or type "file")
 	      (if (and default-directory
 		       (buffer-file-name (buffer-base-buffer)) same-directory?)
 		  (if (eq org-link-file-path-type 'adaptive)
@@ -2571,6 +2580,18 @@ file's directory then expand relative links."
 		result)
 	      (if description (concat "[" description "]") "")))))
 
+(defun org-babel-result-to-file-attachment (result &optional description)
+  "Convert RESULT into an Org link after moving the provided path to the current node's attachment directory as reported by `org-attach-dir', using the optional DESCRIPTION if provided.
+See `org-babel-result-to-file' for further link generation details."
+
+  (let* ((dir (file-relative-name
+               (org-attach-dir t)))
+         (filename (file-name-nondirectory result))
+         (newname (format "%s/%s" dir filename)))
+
+    (rename-file result newname t)
+    (org-babel-result-to-file filename description "attachment")))
+
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
   "Comment out region using the inline `==' or `: ' org example quote."
   (interactive "*r")
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index e8e8ade83..9f180255a 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -323,6 +323,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (call-interactively command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
-- 
2.32.0.windows.2

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

[Timothy]

[-- Attachment #1.1: Type: text/plain, Size: 717 bytes --]

Hi Ryan,

Thanks for submitting your first patch to Org! 🎉

Regarding the purpose of the patch, I take it this is useful with source blocks
that return a file patch and should be added as an attachment? Would you mind
expanding on this use case? Oh, and `org-attach-dir' doesn’t show up a function or
variable for me, I take it you meant something else?

Lastly, with your commit message, you’ve clearly looked at the requested commit
message format 😊. There’s the odd typo (“theh”) and the line length is too
long. It’s good if you hard line-wrap to 72 characters (if you use Magit, this
should happen automatically FYI). Other than that it looks good to me.

All the best,
Timothy

[-- Attachment #1.2: Type: text/html, Size: 4019 bytes --]

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

Great. Yeah I tried to adhere to the commit format the best I could.
org-attach-dir is a function for me (latest org pulled using straight.el)
org/lisp/org-attach.el:327.

The primary use case is src blocks that generate files, in my case usually
gnuplot or graphviz, and return a file path. With a collection of org files
in a directory, organization can get messy, and creating an organizational
scheme essentially recreates the attachment directory design.

Another approach would be to instead only modify org to have hooks (or any
other callback mechanism really) that are run on link insertion and have
access to the result-params for the block. The rest of this could then be a
separate package easily enough. Would that be a better approach as it would
allow the org core to not be so tightly coupled to org-attach?

I'm using magit; I just don't normally restrain myself to the line length.
I'll make sure to do that for submitted patches here.

In terms of this mailing list and overall contribution process, how best to
remedy things for the patch? Just modify it and reply with the modified
patch as an attachment?

On Tue, Aug 31, 2021 at 4:24 AM Timothy <tecosaur@gmail.com> wrote:

> Hi Ryan,
>
> Thanks for submitting your first patch to Org! 🎉
>
> Regarding the purpose of the patch, I take it this is useful with source
> blocks that return a file patch and should be added as an attachment? Would
> you mind expanding on this use case? Oh, and org-attach-dir doesn’t show
> up a function or variable for me, I take it you meant something else?
>
> Lastly, with your commit message, you’ve clearly looked at the requested
> commit message format 😊. There’s the odd typo (“theh”) and the line length
> is too long. It’s good if you hard line-wrap to 72 characters (if you use
> Magit, this should happen automatically FYI). Other than that it looks
> good to me.
>
> All the best,
> *Timothy*
>
> * From*: Ryan Scott <%22Ryan+Scott%22+%3Cryan@vicarious-living.com%3E>
> * Subject*: New source block results option for attaching file to node
> * To*: emacs-orgmode@gnu.org
> <%22emacs-orgmode@gnu.org%22+%3Cemacs-orgmode@gnu.org%3E>
> * Date*: Thu, 26 Aug 2021 16:48:50 +0800
> An additional option for use with ":results file" that moves the returned
> path to the node attachment directory (as returned by org-attach-dir),
> creating it if necessary.
>
> First time submitting a patch. Any feedback is appreciated.
>
> -ryan
>
>

[-- Attachment #2: Type: text/html, Size: 5289 bytes --]

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

[Ihor Radchenko]

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

The patch looks fine for me except a typo:

> +          by the source block to the nodes attachmen directory (as
                                              ^attachment

> org-attach-dir is a function for me (latest org pulled using straight.el)
> org/lisp/org-attach.el:327.

Timothy probably does not have (require 'org-attach) in his personal
config. However, it should not be an issue for your patch with the
autoload you added.

> The primary use case is src blocks that generate files, in my case usually
> gnuplot or graphviz, and return a file path. With a collection of org files
> in a directory, organization can get messy, and creating an organizational
> scheme essentially recreates the attachment directory design.

I am also using attach directories for gnuplot output. Your approach is
fine, but what about input directory? I find it a bit awkward to store
input files alongside with the main .org file, while keeping the output
images as attachments.

I personally prefer to set the working dir for gnuplot like

#+begin_src gnuplot :dir (org-attach-dir)

With my approach, both the input and output files are going to be in the
attach dir.

I even go as far as making attach dir my default directory of all the
code blocks.

Though your patch may be useful when input directory is read-only or
even remote.

> Another approach would be to instead only modify org to have hooks (or any
> other callback mechanism really) that are run on link insertion and have
> access to the result-params for the block. The rest of this could then be a
> separate package easily enough. Would that be a better approach as it would
> allow the org core to not be so tightly coupled to org-attach?

org-attach is in the Org core. It should not be a problem supporting
org-attach in org babel.

> I'm using magit; I just don't normally restrain myself to the line length.
> I'll make sure to do that for submitted patches here.

You may find flycheck-mode useful to hint things like line length.

> In terms of this mailing list and overall contribution process, how best to
> remedy things for the patch? Just modify it and reply with the modified
> patch as an attachment?

Yep. Just submit the updated patch. Preferably, also add [PATCH] at the
beginning of the message topic. Then, the patch will also be shown in
updates.orgmode.org

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 3040 bytes --]

I hadn't thought about input directories much as my usage of
graphviz/gnuplots is through [essentially] DSLs that I made for them, so
the blocks are actually elisp.
Perhaps a convenient way of setting the working directory to the attachment
directory per-block makes sense?

My own personal coding style doesn't include a hard line limit, so it just
wasn't on my mind.

I'll get my patch updated and submitted shortly. Thanks for the help!

On Wed, Sep 1, 2021 at 7:44 AM Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> The patch looks fine for me except a typo:
>
> > +          by the source block to the nodes attachmen directory (as
>                                               ^attachment
>
> > org-attach-dir is a function for me (latest org pulled using straight.el)
> > org/lisp/org-attach.el:327.
>
> Timothy probably does not have (require 'org-attach) in his personal
> config. However, it should not be an issue for your patch with the
> autoload you added.
>
> > The primary use case is src blocks that generate files, in my case
> usually
> > gnuplot or graphviz, and return a file path. With a collection of org
> files
> > in a directory, organization can get messy, and creating an
> organizational
> > scheme essentially recreates the attachment directory design.
>
> I am also using attach directories for gnuplot output. Your approach is
> fine, but what about input directory? I find it a bit awkward to store
> input files alongside with the main .org file, while keeping the output
> images as attachments.
>
> I personally prefer to set the working dir for gnuplot like
>
> #+begin_src gnuplot :dir (org-attach-dir)
>
> With my approach, both the input and output files are going to be in the
> attach dir.
>
> I even go as far as making attach dir my default directory of all the
> code blocks.
>
> Though your patch may be useful when input directory is read-only or
> even remote.
>
> > Another approach would be to instead only modify org to have hooks (or
> any
> > other callback mechanism really) that are run on link insertion and have
> > access to the result-params for the block. The rest of this could then
> be a
> > separate package easily enough. Would that be a better approach as it
> would
> > allow the org core to not be so tightly coupled to org-attach?
>
> org-attach is in the Org core. It should not be a problem supporting
> org-attach in org babel.
>
> > I'm using magit; I just don't normally restrain myself to the line
> length.
> > I'll make sure to do that for submitted patches here.
>
> You may find flycheck-mode useful to hint things like line length.
>
> > In terms of this mailing list and overall contribution process, how best
> to
> > remedy things for the patch? Just modify it and reply with the modified
> > patch as an attachment?
>
> Yep. Just submit the updated patch. Preferably, also add [PATCH] at the
> beginning of the message topic. Then, the patch will also be shown in
> updates.orgmode.org
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 3832 bytes --]

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 3792 bytes --]

Updated patch; fixed typos and wrapped the commit message to 72 characters

Thinking about the input files issue some more, I wonder if an attach
keyword wouldn't better function as setting the working directory to the
node's attachment directory and then fixing up any inserted link to use
"attachment:" if it's relative to that.

I've been working with this for a bit in my own workflow and it's been
useful, but I'll see about maybe experimenting with some other approaches
as well. Moving the referenced file to the attachment directory never felt
completely right to me.

On Wed, Sep 1, 2021 at 1:01 PM Ryan Scott <ryan@vicarious-living.com> wrote:

> I hadn't thought about input directories much as my usage of
> graphviz/gnuplots is through [essentially] DSLs that I made for them, so
> the blocks are actually elisp.
> Perhaps a convenient way of setting the working directory to the
> attachment directory per-block makes sense?
>
> My own personal coding style doesn't include a hard line limit, so it just
> wasn't on my mind.
>
> I'll get my patch updated and submitted shortly. Thanks for the help!
>
> On Wed, Sep 1, 2021 at 7:44 AM Ihor Radchenko <yantar92@gmail.com> wrote:
>
>> Ryan Scott <ryan@vicarious-living.com> writes:
>>
>> The patch looks fine for me except a typo:
>>
>> > +          by the source block to the nodes attachmen directory (as
>>                                               ^attachment
>>
>> > org-attach-dir is a function for me (latest org pulled using
>> straight.el)
>> > org/lisp/org-attach.el:327.
>>
>> Timothy probably does not have (require 'org-attach) in his personal
>> config. However, it should not be an issue for your patch with the
>> autoload you added.
>>
>> > The primary use case is src blocks that generate files, in my case
>> usually
>> > gnuplot or graphviz, and return a file path. With a collection of org
>> files
>> > in a directory, organization can get messy, and creating an
>> organizational
>> > scheme essentially recreates the attachment directory design.
>>
>> I am also using attach directories for gnuplot output. Your approach is
>> fine, but what about input directory? I find it a bit awkward to store
>> input files alongside with the main .org file, while keeping the output
>> images as attachments.
>>
>> I personally prefer to set the working dir for gnuplot like
>>
>> #+begin_src gnuplot :dir (org-attach-dir)
>>
>> With my approach, both the input and output files are going to be in the
>> attach dir.
>>
>> I even go as far as making attach dir my default directory of all the
>> code blocks.
>>
>> Though your patch may be useful when input directory is read-only or
>> even remote.
>>
>> > Another approach would be to instead only modify org to have hooks (or
>> any
>> > other callback mechanism really) that are run on link insertion and have
>> > access to the result-params for the block. The rest of this could then
>> be a
>> > separate package easily enough. Would that be a better approach as it
>> would
>> > allow the org core to not be so tightly coupled to org-attach?
>>
>> org-attach is in the Org core. It should not be a problem supporting
>> org-attach in org babel.
>>
>> > I'm using magit; I just don't normally restrain myself to the line
>> length.
>> > I'll make sure to do that for submitted patches here.
>>
>> You may find flycheck-mode useful to hint things like line length.
>>
>> > In terms of this mailing list and overall contribution process, how
>> best to
>> > remedy things for the patch? Just modify it and reply with the modified
>> > patch as an attachment?
>>
>> Yep. Just submit the updated patch. Preferably, also add [PATCH] at the
>> beginning of the message topic. Then, the patch will also be shown in
>> updates.orgmode.org
>>
>> Best,
>> Ihor
>>
>

[-- Attachment #1.2: Type: text/html, Size: 4900 bytes --]

[-- Attachment #2: 0001-ob-core-Added-option-for-attaching-file-from-babel-s.patch --]
[-- Type: application/octet-stream, Size: 4922 bytes --]

From cbe97587df4b660f56e420015222e4f3559588a2 Mon Sep 17 00:00:00 2001
From: "Ryan C. Scott" <ryan@5pmcasual.com>
Date: Wed, 25 Aug 2021 02:15:09 -0700
Subject: [PATCH] ob-core: Added option for attaching file from babel
src block result

* ob-core.el (org-babel-format-result): Added `:results-param' option
`attach' which when used with `file' will cause the resultant file path
to be moved to the `org-attach-dir' for the node.
(org-babel-result-to-file): Added optional 3rd parameter `type' which
will be used in place of "file" in link type, if present.
(org-babel-result-to-file-attachment): New function that moves the file
at the provided `result' path to the nodes attachment directory,
creating it as necessary using `org-attach-dir'.  A new function was
created to adhere to the design of the API and not complicated
`org-babel-result-to-file' with the movement to the attachment
directory.

* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-result-to-file-attachment').
---
 lisp/ob-core.el    | 31 ++++++++++++++++++++++++++-----
 lisp/org-attach.el |  1 +
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 857e03e55..c1d1b77e0 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2188,6 +2188,10 @@ silent -- no results are inserted into the Org buffer but
 file ---- the results are interpreted as a file path, and are
           inserted into the buffer using the Org file syntax.
 
+attach -- used with `file', will move the file at the path returned
+          by the source block to the nodes attachment directory (as
+          reported by `org-attach-dir'.
+
 list ---- the results are interpreted as an Org list.
 
 raw ----- results are added directly to the Org file.  This is
@@ -2241,9 +2245,13 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result (if (member "attach" result-params)
+                            (org-babel-result-to-file-attachment
+                             result
+                             (org-babel--file-desc (nth 2 info) result))
+                          (org-babel-result-to-file
+			   result
+			   (org-babel--file-desc (nth 2 info) result))))))
 	((listp result))
 	(t (setq result (format "%S" result))))
   (if (and result-params (member "silent" result-params))
@@ -2548,7 +2556,7 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
 file's directory then expand relative links."
@@ -2559,7 +2567,8 @@ file's directory then expand relative links."
 			    (expand-file-name
 			     (file-name-directory
 			      (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
+      (format "[[%s:%s]%s]"
+              (or type "file")
 	      (if (and default-directory
 		       (buffer-file-name (buffer-base-buffer)) same-directory?)
 		  (if (eq org-link-file-path-type 'adaptive)
@@ -2571,6 +2580,18 @@ file's directory then expand relative links."
 		result)
 	      (if description (concat "[" description "]") "")))))
 
+(defun org-babel-result-to-file-attachment (result &optional description)
+  "Convert RESULT into an Org link after moving the provided path to the current node's attachment directory as reported by `org-attach-dir', using the optional DESCRIPTION if provided.
+See `org-babel-result-to-file' for further link generation details."
+
+  (let* ((dir (file-relative-name
+               (org-attach-dir t)))
+         (filename (file-name-nondirectory result))
+         (newname (format "%s/%s" dir filename)))
+
+    (rename-file result newname t)
+    (org-babel-result-to-file filename description "attachment")))
+
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
   "Comment out region using the inline `==' or `: ' org example quote."
   (interactive "*r")
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index e8e8ade83..9f180255a 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -323,6 +323,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (call-interactively command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
-- 
2.32.0.windows.2

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

[Greg Minshall]

Ryan, et al.,

i'm not entirely following the discussion, as i don't use "attaching".
but, fwiw, if i did, i can imagine wanting to have input files and
output files in separate directories.  (for ease in "make clean", if for
no other conceptual reason.)  (but, probably i don't understand.)

cheers, Greg

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

[Ihor Radchenko]

Greg Minshall <minshall@umich.edu> writes:

> i can imagine wanting to have input files and
> output files in separate directories.  (for ease in "make clean", if for
> no other conceptual reason.)  (but, probably i don't understand.)

Makes sense. Currently, there is :dir header arg to set working
directory (aka input files directory). Maybe we can introduce something
like :results-dir header arg to set the output directory? It's value can
be a directory path or symbol 'attach.

`:results file :results-dir 'attach` will be equivalent of
`:results file attach` in the patch proposed by Ryan Scott.

WDYT?

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

That's starting to sound pretty good.

It might make sense to fix up inserted "file:" links that are under the
attachment directory to be "attachment:" style links by default anyway, no?
Then just being able to set the working directory to the attachment
directory easily would get the rest of the way there.

So I suppose that would then mean having the :dir header accept the symbol
`attach' or something like that?
I'll play around and see what that looks like.

On Thu, Sep 2, 2021 at 8:09 PM Ihor Radchenko <yantar92@gmail.com> wrote:

> Greg Minshall <minshall@umich.edu> writes:
>
> > i can imagine wanting to have input files and
> > output files in separate directories.  (for ease in "make clean", if for
> > no other conceptual reason.)  (but, probably i don't understand.)
>
> Makes sense. Currently, there is :dir header arg to set working
> directory (aka input files directory). Maybe we can introduce something
> like :results-dir header arg to set the output directory? It's value can
> be a directory path or symbol 'attach.
>
> `:results file :results-dir 'attach` will be equivalent of
> `:results file attach` in the patch proposed by Ryan Scott.
>
> WDYT?
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 1746 bytes --]

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

[Ihor Radchenko]

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

> It might make sense to fix up inserted "file:" links that are under the
> attachment directory to be "attachment:" style links by default anyway, no?
> Then just being able to set the working directory to the attachment
> directory easily would get the rest of the way there.

I am not sure. If the user explicitly states that :dir is the attachment
dir, it would make sense. However, what if the :dir is set explicitly
like below?

* Headline
:PROPERTIES:
:DIR: /actual/literal/path/to/attachment/dir
:END:

#+begin_src emacs-lisp :dir /actual/literal/path/to/attachment/dir
...

#+RESULTS:
attachment:...

The results will be indeed inside the attachment directory. However, the
:DIR: property may be changed at some point and the existing attachment:
link will not point to real file.

> So I suppose that would then mean having the :dir header accept the symbol
> `attach' or something like that?
> I'll play around and see what that looks like.

The above example should lead to more expected behaviour if the user
explicitly states that :dir is the attachment dir (even if it is going
to be changed in future):

* Headline
:PROPERTIES:
:DIR: /actual/literal/path/to/attachment/dir
:END:

#+begin_src emacs-lisp :dir 'attach
...

#+RESULTS:
attachment:...

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

Yeah your second example is what I'm thinking. It makes this all a fairly
concise extension of that existing mechanism and does away with the file
move after execution.

On Sun, Sep 5, 2021, 06:21 Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> > It might make sense to fix up inserted "file:" links that are under the
> > attachment directory to be "attachment:" style links by default anyway,
> no?
> > Then just being able to set the working directory to the attachment
> > directory easily would get the rest of the way there.
>
> I am not sure. If the user explicitly states that :dir is the attachment
> dir, it would make sense. However, what if the :dir is set explicitly
> like below?
>
> * Headline
> :PROPERTIES:
> :DIR: /actual/literal/path/to/attachment/dir
> :END:
>
> #+begin_src emacs-lisp :dir /actual/literal/path/to/attachment/dir
> ...
>
> #+RESULTS:
> attachment:...
>
> The results will be indeed inside the attachment directory. However, the
> :DIR: property may be changed at some point and the existing attachment:
> link will not point to real file.
>
> > So I suppose that would then mean having the :dir header accept the
> symbol
> > `attach' or something like that?
> > I'll play around and see what that looks like.
>
> The above example should lead to more expected behaviour if the user
> explicitly states that :dir is the attachment dir (even if it is going
> to be changed in future):
>
> * Headline
> :PROPERTIES:
> :DIR: /actual/literal/path/to/attachment/dir
> :END:
>
> #+begin_src emacs-lisp :dir 'attach
> ...
>
> #+RESULTS:
> attachment:...
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 2204 bytes --]

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 3014 bytes --]

Okay, Had some time to put into this. Much happier with this approach as it
doesn't require any file moving and generally leaves src blocks to their
own devices.
The short version is that specifying ":dir 'attach" for a block uses the
directory from (org-attach-dir) as its working directory and any generated
path that is a descendant of that directory will be converted to an
"attachment:" link.

ob-core.el/babel: Special handling for attachment links in src blocks

* ob-core.el (org-babel-execute-src-block): Specifying the symbol
'attach` as the value of the `:dir' header now functions as
":dir (org-attach-dir)"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `DEFAULT-DIRECTORY' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file' when header `:dir' is set to symbol
`attach'
(org-babel-load-in-session, org-babel-initiate-session) ":dir 'attach"
sets `default-directory' with "(org-attach-dir t)"
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-execute-src-block').

On Sun, Sep 5, 2021 at 6:56 AM Ryan Scott <ryan@vicarious-living.com> wrote:

> Yeah your second example is what I'm thinking. It makes this all a fairly
> concise extension of that existing mechanism and does away with the file
> move after execution.
>
> On Sun, Sep 5, 2021, 06:21 Ihor Radchenko <yantar92@gmail.com> wrote:
>
>> Ryan Scott <ryan@vicarious-living.com> writes:
>>
>> > It might make sense to fix up inserted "file:" links that are under the
>> > attachment directory to be "attachment:" style links by default anyway,
>> no?
>> > Then just being able to set the working directory to the attachment
>> > directory easily would get the rest of the way there.
>>
>> I am not sure. If the user explicitly states that :dir is the attachment
>> dir, it would make sense. However, what if the :dir is set explicitly
>> like below?
>>
>> * Headline
>> :PROPERTIES:
>> :DIR: /actual/literal/path/to/attachment/dir
>> :END:
>>
>> #+begin_src emacs-lisp :dir /actual/literal/path/to/attachment/dir
>> ...
>>
>> #+RESULTS:
>> attachment:...
>>
>> The results will be indeed inside the attachment directory. However, the
>> :DIR: property may be changed at some point and the existing attachment:
>> link will not point to real file.
>>
>> > So I suppose that would then mean having the :dir header accept the
>> symbol
>> > `attach' or something like that?
>> > I'll play around and see what that looks like.
>>
>> The above example should lead to more expected behaviour if the user
>> explicitly states that :dir is the attachment dir (even if it is going
>> to be changed in future):
>>
>> * Headline
>> :PROPERTIES:
>> :DIR: /actual/literal/path/to/attachment/dir
>> :END:
>>
>> #+begin_src emacs-lisp :dir 'attach
>> ...
>>
>> #+RESULTS:
>> attachment:...
>>
>> Best,
>> Ihor
>>
>

[-- Attachment #1.2: Type: text/html, Size: 4060 bytes --]

[-- Attachment #2: 0001-ob-core.el-babel-Special-handling-for-attachment-lin.patch --]
[-- Type: application/octet-stream, Size: 6789 bytes --]

From 4d489da66cf6f4d44a320e2d3ccb7025aa8ada6d Mon Sep 17 00:00:00 2001
From: "Ryan C. Scott" <ryan@5pmcasual.com>
Date: Thu, 9 Sep 2021 17:19:34 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
src block

* ob-core.el (org-babel-execute-src-block): Specifying the symbol
'attach` as the value of the `:dir' header now functions as
":dir (org-attach-dir)"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `DEFAULT-DIRECTORY' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file' when header `:dir' is set to symbol
`attach'
(org-babel-load-in-session, org-babel-initiate-session) ":dir 'attach"
sets `default-directory' with "(org-attach-dir t)"
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-execute-src-block').
---
 lisp/ob-core.el    | 72 +++++++++++++++++++++++++++++++---------------
 lisp/org-attach.el |  1 +
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 384c06c9a..90e454319 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -704,6 +704,9 @@ block."
 		 (mkdirp (cdr (assq :mkdirp params)))
 		 (default-directory
 		   (cond
+                    ((eq dir 'attach)
+                     (file-name-as-directory
+                      (org-attach-dir t)))
 		    ((not dir) default-directory)
 		    ((member mkdirp '("no" "nil" nil))
 		     (file-name-as-directory (expand-file-name dir)))
@@ -925,7 +928,10 @@ session."
          (session (cdr (assq :session params)))
 	 (dir (cdr (assq :dir params)))
 	 (default-directory
-	   (or (and dir (file-name-as-directory dir)) default-directory))
+	   (or (and dir (if (eq dir 'attach)
+                            (org-attach-dir t)
+                          (file-name-as-directory dir)))
+               default-directory))
 	 (cmd (intern (concat "org-babel-load-session:" lang))))
     (unless (fboundp cmd)
       (error "No org-babel-load-session function for %s!" lang))
@@ -946,7 +952,10 @@ the session.  Copy the body of the code block to the kill ring."
          (session (cdr (assq :session params)))
 	 (dir (cdr (assq :dir params)))
 	 (default-directory
-	   (or (and dir (file-name-as-directory dir)) default-directory))
+	   (or (and dir (if (eq dir 'attach)
+                            (org-attach-dir t)
+                          (file-name-as-directory dir)))
+               default-directory))
 	 (init-cmd (intern (format "org-babel-%s-initiate-session" lang)))
 	 (prep-cmd (intern (concat "org-babel-prep-session:" lang))))
     (when (and (stringp session) (string= session "none"))
@@ -2241,9 +2250,12 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result (let ((params (nth 2 info)))
+                          (org-babel-result-to-file
+			   result
+			   (org-babel--file-desc params result)
+                           (when (equal (cdr (assq :dir params)) 'attach)
+                             'attachment))))))
 	((listp result))
 	(t (setq result (format "%S" result))))
   (if (and result-params (member "silent" result-params))
@@ -2548,27 +2560,41 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			    (expand-file-name
-			     (file-name-directory
-			      (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        (file-name-directory
+			         base-file-name))))))
+           (request-attachment (eq type 'attachment))
+           (attach-dir-len (when request-attachment (length default-directory)))
+           (in-attach-dir (when (and request-attachment (> (length result-file-name) attach-dir-len))
+                            (string=
+                             (substring result-file-name 0 attach-dir-len)
+                             default-directory))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ('attachment "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index f18453103..b06b85360 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -323,6 +323,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (call-interactively command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
-- 
2.32.0.windows.2

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

[Timothy]

[-- Attachment #1.1: Type: text/plain, Size: 234 bytes --]

Hi Ryan,

I’ve just had a glance, but this looks much better to me than what was proposed
earlier 👍. Hopefully we’ll be able to get some feedback on this from others,
and then see it merged 🙂.

All the best,
Timothy

[-- Attachment #1.2: Type: text/html, Size: 7242 bytes --]

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

[Ihor Radchenko]

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

>  	 (default-directory
> -	   (or (and dir (file-name-as-directory dir)) default-directory))
> +	   (or (and dir (if (eq dir 'attach)
> +                            (org-attach-dir t)
> +                          (file-name-as-directory dir)))
> +               default-directory))

This does not always work.  Some ob-*.el code (namely, ob-lisp have the
following:

(let ((dir (if (assq :dir params)
               (cdr (assq :dir params))
              default-directory)))

As you can see, :dir parameter is overriding default-directory.  If :dir
is set to symbol 'attach, execution will fail.

I think that you also need to override :dir in the parameter list and
put actual path to attachment dir instead of symbol.  That way, we will
not need to change every possible ob-*.el implementation to account for
new 'attach option.

Also, marking this as patch.

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

Would it be better then as a new option entirely that sets the default
directory to the attachment directory and results in attachment links for
any inserted paths that are under that?

The attachment link detection could possibly be default behavior for link
insertion, but i can imagine that might have broader implications.

I also found and have a fix for my patch where the 'attach symbol gets
converted to a string when using #+call to call a block that is defined
with this option, which felt a little awkward in the code.

I'll try this as a standalone option and see how that feels. That would
carve out space for other options in handling attachments.

On Sat, Oct 2, 2021, 01:31 Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> >        (default-directory
> > -        (or (and dir (file-name-as-directory dir)) default-directory))
> > +        (or (and dir (if (eq dir 'attach)
> > +                            (org-attach-dir t)
> > +                          (file-name-as-directory dir)))
> > +               default-directory))
>
> This does not always work.  Some ob-*.el code (namely, ob-lisp have the
> following:
>
> (let ((dir (if (assq :dir params)
>                (cdr (assq :dir params))
>               default-directory)))
>
> As you can see, :dir parameter is overriding default-directory.  If :dir
> is set to symbol 'attach, execution will fail.
>
> I think that you also need to override :dir in the parameter list and
> put actual path to attachment dir instead of symbol.  That way, we will
> not need to change every possible ob-*.el implementation to account for
> new 'attach option.
>
> Also, marking this as patch.
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 2402 bytes --]

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

[Christopher M. Miles]

[-- Attachment #1.1: Type: text/plain, Size: 857 bytes --]



Ihor Radchenko <yantar92@gmail.com> writes:

> Greg Minshall <minshall@umich.edu> writes:
>
>> i can imagine wanting to have input files and
>> output files in separate directories.  (for ease in "make clean", if for
>> no other conceptual reason.)  (but, probably i don't understand.)

I agree with this thought. We should separate two directories.

>
> Makes sense. Currently, there is :dir header arg to set working
> directory (aka input files directory). Maybe we can introduce something
> like :results-dir header arg to set the output directory? It's value can
> be a directory path or symbol 'attach.
>
> `:results file :results-dir 'attach` will be equivalent of
> `:results file attach` in the patch proposed by Ryan Scott.
>
> WDYT?

I agree with this idea. Use ~:results-dir 'attach~ is better.

Will the patch be updated?

>
> Best,
> Ihor



[-- Attachment #1.2: Type: text/html, Size: 1515 bytes --]

[-- Attachment #2: Type: text/plain, Size: 282 bytes --]


<#secure method=pgpmime mode=sign>
-- 
[ stardiviner ]
       I try to make every word tell the meaning that I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]

I've been working through a few different approaches. What's shaping up is
something more general, having a special value for directory parameters
(i.e. 'attach) and auto-detection of link paths that are in the attachment
directory.
The latest iterations don't move any files around, so can't actually
enforce the output directory. That makes it safer overall as with my
initial patch if you were to return a path to something you *didn't* want
moved to your attachment directory you might get very surprising results.

I'll post a new patch with a different approach in a little bit.

On Mon, Oct 4, 2021 at 5:06 PM Christopher M. Miles <numbchild@gmail.com>
wrote:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
> Greg Minshall <minshall@umich.edu> writes:
>>
>> i can imagine wanting to have input files and output files in separate
>>> directories. (for ease in "make clean", if for no other conceptual reason.)
>>> (but, probably i don't understand.)
>>>
>> I agree with this thought. We should separate two directories.
>
> Makes sense. Currently, there is :dir header arg to set working directory
>> (aka input files directory). Maybe we can introduce something like
>> :results-dir header arg to set the output directory? It's value can be a
>> directory path or symbol 'attach.
>>
>> `:results file :results-dir 'attach` will be equivalent of `:results file
>> attach` in the patch proposed by Ryan Scott.
>>
>> WDYT?
>>
> I agree with this idea. Use :results-dir 'attach is better.
>
> Will the patch be updated?
>
> Best, Ihor
>>
>
> <#secure method=pgpmime mode=sign>
> --
> [ stardiviner ]
>        I try to make every word tell the meaning that I want to express.
>
>        Blog: https://stardiviner.github.io/
>        IRC(freenode): stardiviner, Matrix: stardiviner
>        GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
>

[-- Attachment #2: Type: text/html, Size: 3264 bytes --]

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

[Christopher M. Miles]

[-- Attachment #1.1: Type: text/plain, Size: 2791 bytes --]



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

> I've been working through a few different approaches. What's shaping up is something more general, having a special value
> for directory parameters (i.e. 'attach) and auto-detection of link paths that are in the attachment directory.
> The latest iterations don't move any files around, so can't actually enforce the output directory. That makes it safer
> overall as with my initial patch if you were to return a path to something you didn't want moved to your attachment
> directory you might get very surprising results.

I used to created a similar function extension called "ob-clojure-literate.el" for ob-clojure.el.
One functionality is to save plotting result image to a header argument :dir specified directory.
But it's a little complicated. Then later I found another solution through *header argument evaluation*:

#+begin_src clojure :results file link :dir "data/images" :file "ob-clojure-incanter-move.png" :var dir=(concat (file-name-directory (buffer-file-name)) "data/images/")
(use '(incanter core io charts stats))
(import '(java.io File))

(def hist (histogram (sample-normal 1000)))
(save hist "ob-clojure-incanter-move.png")
(.renameTo (File. "ob-clojure-incanter-move.png") (File. (str dir "ob-clojure-incanter-move.png")))
#+end_src

But this code has a disadvantage:

This solution breaks Literate Programming tangling concept by introduced un-reimplemented variable ~dir~.

>
> I'll post a new patch with a different approach in a little bit.
>
> On Mon, Oct 4, 2021 at 5:06 PM Christopher M. Miles <numbchild@gmail.com> wrote:
>
>  Ihor Radchenko <yantar92@gmail.com> writes: 
>
>  Greg Minshall <minshall@umich.edu> writes: 
>
>  i can imagine wanting to have input files and output files in separate directories. (for ease in "make
>  clean", if for no other conceptual reason.) (but, probably i don't understand.) 
>
>  I agree with this thought. We should separate two directories. 
>
>  Makes sense. Currently, there is :dir header arg to set working directory (aka input files directory). Maybe we
>  can introduce something like :results-dir header arg to set the output directory? It's value can be a directory
>  path or symbol 'attach. 
>
>  `:results file :results-dir 'attach` will be equivalent of `:results file attach` in the patch proposed by Ryan
>  Scott. 
>
>  WDYT? 
>
>  I agree with this idea. Use :results-dir 'attach is better. 
>
>  Will the patch be updated? 
>
>  Best, Ihor 
>
>  <#secure method=pgpmime mode=sign>
>  -- 
>  [ stardiviner ]
>         I try to make every word tell the meaning that I want to express.
>
>         Blog: https://stardiviner.github.io/
>         IRC(freenode): stardiviner, Matrix: stardiviner
>         GPG: F09F650D7D674819892591401B5DF1C95AE89AC3



[-- Attachment #1.2: Type: text/html, Size: 4990 bytes --]

[-- Attachment #2: Type: text/plain, Size: 282 bytes --]


<#secure method=pgpmime mode=sign>
-- 
[ stardiviner ]
       I try to make every word tell the meaning that I want to express.

       Blog: https://stardiviner.github.io/
       IRC(freenode): stardiviner, Matrix: stardiviner
       GPG: F09F650D7D674819892591401B5DF1C95AE89AC3

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 2324 bytes --]

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.

On Mon, Oct 4, 2021 at 6:05 PM Ryan Scott <ryan@vicarious-living.com> wrote:

> I've been working through a few different approaches. What's shaping up is
> something more general, having a special value for directory parameters
> (i.e. 'attach) and auto-detection of link paths that are in the attachment
> directory.
> The latest iterations don't move any files around, so can't actually
> enforce the output directory. That makes it safer overall as with my
> initial patch if you were to return a path to something you *didn't* want
> moved to your attachment directory you might get very surprising results.
>
> I'll post a new patch with a different approach in a little bit.
>
> On Mon, Oct 4, 2021 at 5:06 PM Christopher M. Miles <numbchild@gmail.com>
> wrote:
>
>> Ihor Radchenko <yantar92@gmail.com> writes:
>>
>> Greg Minshall <minshall@umich.edu> writes:
>>>
>>> i can imagine wanting to have input files and output files in separate
>>>> directories. (for ease in "make clean", if for no other conceptual reason.)
>>>> (but, probably i don't understand.)
>>>>
>>> I agree with this thought. We should separate two directories.
>>
>> Makes sense. Currently, there is :dir header arg to set working directory
>>> (aka input files directory). Maybe we can introduce something like
>>> :results-dir header arg to set the output directory? It's value can be a
>>> directory path or symbol 'attach.
>>>
>>> `:results file :results-dir 'attach` will be equivalent of `:results
>>> file attach` in the patch proposed by Ryan Scott.
>>>
>>> WDYT?
>>>
>> I agree with this idea. Use :results-dir 'attach is better.
>>
>> Will the patch be updated?
>>
>> Best, Ihor
>>>
>>
>> <#secure method=pgpmime mode=sign>
>> --
>> [ stardiviner ]
>>        I try to make every word tell the meaning that I want to express.
>>
>>        Blog: https://stardiviner.github.io/
>>        IRC(freenode): stardiviner, Matrix: stardiviner
>>        GPG: F09F650D7D674819892591401B5DF1C95AE89AC3
>>
>

[-- Attachment #1.2: Type: text/html, Size: 4059 bytes --]

[-- Attachment #2: org-src-block-results-attach-dir.patch --]
[-- Type: application/octet-stream, Size: 8036 bytes --]

From 5f0806f1b49006e5977c8100fa3e336a4e0e1347 Mon Sep 17 00:00:00 2001
From: "Ryan C. Scott" <ryan@5pmcasual.com>
Date: Mon, 4 Oct 2021 19:06:26 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
 src block

* ob-core.el (org-babel-merge-params): Specifying the symbol 'attach`
or string "'attach" as the value of the `:dir' header now functions as
":dir (org-attach-dir nil t) :mkdirp t"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `(org-attach-dir)' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file'
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-merge-params').
---
 doc/org-manual.org |  6 ++++
 etc/ORG-NEWS       |  7 +++++
 lisp/ob-core.el    | 76 +++++++++++++++++++++++++++++++++-------------
 lisp/org-attach.el |  1 +
 4 files changed, 69 insertions(+), 21 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index d34d33561..6ab340a5a 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -17232,6 +17232,12 @@ directory with {{{kbd(M-x cd RET DIRECTORY)}}}, and then not setting
 =dir=.  Under the surface, =dir= simply sets the value of the Emacs
 variable ~default-directory~.  Setting =mkdirp= header argument to
 a non-~nil~ value creates the directory, if necessary.
+Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
+set =dir= to the directory returned by ~(org-attach-dir)~, set =:mkdir
+yes=, and insert any file paths, as when using =:results file=, which
+are under the node's attachment directory using =attachment:= links
+instead of the usual =file:= links. Any returned path outside of the
+attachment directory will use =file:= links as per usual.
 
 For example, to save the plot file in the =Work/= folder of the home
 directory---notice tilde is expanded:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 83a67da96..d0d54cbb5 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -354,6 +354,13 @@ argument is present.  You can also set =:async no= to force it off
 
 Async evaluation is disabled during export.
 
+*** New special value ~'attach~ for src block =:dir= option
+
+Passing the symbol ~attach~ or string ='attach= to the =:dir= option
+of a src block is now equivalent to =:dir (org-attach-dir) :mkdir yes= and any
+file results with a path descended from the attachment directory will use
+=attachment:= style links instead of the standard =file:= link type.
+
 ** Miscellaneous
 *** =org-bibtex= includes =doi= and =url= entries when exporting to BiBTeX
 =doi= and =url= entries have been made optional for some publication
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 384c06c9a..05522ca24 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2241,11 +2241,14 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result
+                 (org-babel-result-to-file
+		  result
+		  (org-babel--file-desc (nth 2 info) result)
+                  'attachment))))
 	((listp result))
 	(t (setq result (format "%S" result))))
+
   (if (and result-params (member "silent" result-params))
       (progn (message (replace-regexp-in-string "%" "%%" (format "%S" result)))
 	     result)
@@ -2548,27 +2551,47 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			    (expand-file-name
-			     (file-name-directory
-			      (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (base-directory (file-name-directory base-file-name))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        base-directory)))))
+           (request-attachment (eq type 'attachment))
+           (attach-dir (when request-attachment
+                         (let ((default-directory base-directory))
+                           (org-attach-dir nil t))))
+           (attach-dir-len (when request-attachment (length attach-dir)))
+           (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))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ((and 'attachment (guard in-attach-dir)) "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
@@ -2698,6 +2721,17 @@ parameters when merging lists."
 				  exports-exclusive-groups
 				  exports
 				  (split-string (or value "")))))
+          ((or '(:dir . attach) '(:dir . "'attach"))
+           (unless (org-id-get)
+             (if (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))))
+           (setq params (append
+                         `((:dir . ,(org-attach-dir nil t))
+                           (:mkdirp . "yes"))
+                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))
 	  ;; Regular keywords: any value overwrites the previous one.
 	  (_ (setq params (cons pair (assq-delete-all (car pair) params)))))))
     ;; Handle `:var' and clear out colnames and rownames for replaced
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index f18453103..b06b85360 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -323,6 +323,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (call-interactively command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
-- 
2.33.0.windows.2

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

[Ihor Radchenko]

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

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 3824 bytes --]

Yeah I just recently updated my fourth and realized there were some
necessary conflicts to resolve with the latest.

I'll take a look at addressing these issues and submit an updated patch.
Surprised I missed a calling location and secretly hope that it was added
recently for the sake of my poor ego. :)

With all of the layers and assumptions/behaviors inherent in the way src
block parameters are handled, do you feel like this is an approach that can
work, or should it be abandoned?

Thanks for looking into this.

On Thu, Apr 21, 2022, 05:46 Ihor Radchenko <yantar92@gmail.com> wrote:

> 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
>

[-- Attachment #2: Type: text/html, Size: 4957 bytes --]

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

[Ihor Radchenko]

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

> With all of the layers and assumptions/behaviors inherent in the way src
> block parameters are handled, do you feel like this is an approach that can
> work, or should it be abandoned?

I am not sure if I understand your concern.
Your code is reusing the existing convention that current directory is
managed via default-directory by ob-core.el. I do not see any problem
here.

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 709 bytes --]

Great. Just making sure that this particular approach to this feature or
type of feature wasn't fundamentally flawed given the established behavior
of org.
Thanks.

On Thu, Apr 21, 2022 at 11:02 PM Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> > With all of the layers and assumptions/behaviors inherent in the way src
> > block parameters are handled, do you feel like this is an approach that
> can
> > work, or should it be abandoned?
>
> I am not sure if I understand your concern.
> Your code is reusing the existing convention that current directory is
> managed via default-directory by ob-core.el. I do not see any problem
> here.
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 1132 bytes --]

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 1256 bytes --]

I believe I have addressed your feedback, Ihor.
Attached is the latest version of the patch.

   - Merged latest master
   - :post is now handled correctly (verified with example of :post usage
   in the example at https://orgmode.org/manual/Results-of-Evaluation.html)
   - Added "(with quotes)" to help make the NEWS entry clearer
   - Changed the attach directory detection to use a string prefix check

Let me know what you think.

On Thu, Apr 21, 2022 at 11:19 PM Ryan Scott <ryan@vicarious-living.com>
wrote:

> Great. Just making sure that this particular approach to this feature or
> type of feature wasn't fundamentally flawed given the established behavior
> of org.
> Thanks.
>
> On Thu, Apr 21, 2022 at 11:02 PM Ihor Radchenko <yantar92@gmail.com>
> wrote:
>
>> Ryan Scott <ryan@vicarious-living.com> writes:
>>
>> > With all of the layers and assumptions/behaviors inherent in the way src
>> > block parameters are handled, do you feel like this is an approach that
>> can
>> > work, or should it be abandoned?
>>
>> I am not sure if I understand your concern.
>> Your code is reusing the existing convention that current directory is
>> managed via default-directory by ob-core.el. I do not see any problem
>> here.
>>
>> Best,
>> Ihor
>>
>

[-- Attachment #1.2: Type: text/html, Size: 2075 bytes --]

[-- Attachment #2: org-src-block-results-attach-dir.patch --]
[-- Type: application/octet-stream, Size: 8710 bytes --]

From a35bb11cb20240e594902dcd58b14571be2c7fb6 Mon Sep 17 00:00:00 2001
From: Ryan C. Scott <ryan@5pmCasual.com>
Date: Fri, 10 Jun 2022 00:01:37 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
 src block

* ob-core.el (org-babel-merge-params): Specifying the symbol 'attach`
or string "'attach" as the value of the `:dir' header now functions as
":dir (org-attach-dir nil t) :mkdirp t"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `(org-attach-dir)' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file'
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-merge-params').
---
 doc/org-manual.org |  6 ++++
 etc/ORG-NEWS       |  7 ++++
 lisp/ob-core.el    | 80 +++++++++++++++++++++++++++++++++-------------
 lisp/org-attach.el |  1 +
 4 files changed, 71 insertions(+), 23 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 32a45f884..5f9b3353b 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -17541,6 +17541,12 @@ directory with {{{kbd(M-x cd RET DIRECTORY)}}}, and then not setting
 =dir=.  Under the surface, =dir= simply sets the value of the Emacs
 variable ~default-directory~.  Setting =mkdirp= header argument to
 a non-~nil~ value creates the directory, if necessary.
+Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
+set =dir= to the directory returned by ~(org-attach-dir)~, set =:mkdir
+yes=, and insert any file paths, as when using =:results file=, which
+are under the node's attachment directory using =attachment:= links
+instead of the usual =file:= links. Any returned path outside of the
+attachment directory will use =file:= links as per usual.
 
 For example, to save the plot file in the =Work/= folder of the home
 directory---notice tilde is expanded:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 35af94f92..0c88378d6 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -795,6 +795,13 @@ Finally, the closures are only evaluated if they're not overridden for
 a source block. This improves efficiency in cases where the result of
 a compute-expensive closure would otherwise be discarded.
 
+*** New special value ~'attach~ for src block =:dir= option
+
+Passing the symbol ~attach~ or string ="'attach"= (with quotes) to the =:dir=
+option of a src block is now equivalent to =:dir (org-attach-dir) :mkdir yes=
+and any file results with a path descended from the attachment directory will
+use =attachment:= style links instead of the standard =file:= link type.
+
 ** Miscellaneous
 *** =org-bibtex= includes =doi= and =url= entries when exporting to BiBTeX
 =doi= and =url= entries have been made optional for some publication
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 09d6adfe0..faec7ec9a 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -801,7 +801,8 @@ block."
 		    (let ((*this* (if (not file) result
 				    (org-babel-result-to-file
 				     file
-				     (org-babel--file-desc params result)))))
+				     (org-babel--file-desc params result)
+                                     'attachment))))
 		      (setq result (org-babel-ref-resolve post))
 		      (when file
 			(setq result-params (remove "file" result-params))))))
@@ -2298,11 +2299,14 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result
+                 (org-babel-result-to-file
+		  result
+		  (org-babel--file-desc (nth 2 info) result)
+                  'attachment))))
 	((listp result))
 	(t (setq result (format "%S" result))))
+
   (if (and result-params (member "silent" result-params))
       (progn (message (replace-regexp-in-string "%" "%%" (format "%S" result)))
 	     result)
@@ -2605,27 +2609,46 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			      (expand-file-name
-			       (file-name-directory
-			        (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (base-directory (file-name-directory base-file-name))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        base-directory)))))
+           (request-attachment (eq type 'attachment))
+           (attach-dir (when request-attachment
+                         (let ((default-directory base-directory))
+                           (org-attach-dir nil t))))
+           (in-attach-dir (and request-attachment
+                               (string-prefix-p
+                                attach-dir
+                                result-file-name))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ((and 'attachment (guard in-attach-dir)) "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
@@ -2756,10 +2779,21 @@ parameters when merging lists."
 	   (setq exports (funcall merge
 				  exports-exclusive-groups
 				  exports
-				  (split-string
+                                  (split-string
                                    (cond ((and value (functionp value)) (funcall value))
                                          (value value)
                                          (t ""))))))
+          ((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))))
+           (setq params (append
+                         `((:dir . ,(org-attach-dir nil t))
+                           (:mkdirp . "yes"))
+                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))
 	  ;; Regular keywords: any value overwrites the previous one.
 	  (_ (setq params (cons pair (assq-delete-all (car pair) params)))))))
     ;; Handle `:var' and clear out colnames and rownames for replaced
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index 5ee2b84b2..6a061b2b3 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -324,6 +324,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (command-execute command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
-- 
2.33.0.windows.2

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

[Ihor Radchenko]

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

> I believe I have addressed your feedback, Ihor.
> Attached is the latest version of the patch.
>
>    - Merged latest master
>    - :post is now handled correctly (verified with example of :post usage
>    in the example at https://orgmode.org/manual/Results-of-Evaluation.html)
>    - Added "(with quotes)" to help make the NEWS entry clearer
>    - Changed the attach directory detection to use a string prefix check
>
> Let me know what you think.

Thanks for the updated patch!

> +          ((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))))
> +           (setq params (append
> +                         `((:dir . ,(org-attach-dir nil t))
> +                           (:mkdirp . "yes"))
> +                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))

Note that entry does not need :ID: property to have an attachment dir.
There is also :DIR: property.

Also, it would be useful to add a test. See test-ob-core/dir-mkdirp in
testing/lisp/test-ob.el

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 1883 bytes --]

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.

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

Thanks,
  -ryan

On Fri, Jun 10, 2022 at 9:31 PM Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> > I believe I have addressed your feedback, Ihor.
> > Attached is the latest version of the patch.
> >
> >    - Merged latest master
> >    - :post is now handled correctly (verified with example of :post usage
> >    in the example at
> https://orgmode.org/manual/Results-of-Evaluation.html)
> >    - Added "(with quotes)" to help make the NEWS entry clearer
> >    - Changed the attach directory detection to use a string prefix check
> >
> > Let me know what you think.
>
> Thanks for the updated patch!
>
> > +          ((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))))
> > +           (setq params (append
> > +                         `((:dir . ,(org-attach-dir nil t))
> > +                           (:mkdirp . "yes"))
> > +                         (assq-delete-all :dir (assq-delete-all :mkdir
> params)))))
>
> Note that entry does not need :ID: property to have an attachment dir.
> There is also :DIR: property.
>
> Also, it would be useful to add a test. See test-ob-core/dir-mkdirp in
> testing/lisp/test-ob.el
>
> Best,
> Ihor
>

[-- Attachment #1.2: Type: text/html, Size: 2787 bytes --]

[-- Attachment #2: org-src-block-results-attach-dir.patch --]
[-- Type: application/octet-stream, Size: 11411 bytes --]

From 9b6f1698554316ad8df5f98628564dac9b189276 Mon Sep 17 00:00:00 2001
From: Ryan Scott <ryan@5pmCasual.com>
Date: Fri, 10 Jun 2022 00:01:37 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
 src block

* ob-core.el (org-babel-merge-params): Specifying the symbol 'attach`
or string "'attach" as the value of the `:dir' header now functions as
":dir (org-attach-dir nil t) :mkdirp t"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `(org-attach-dir)' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file'
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-merge-params').
* test-ob.el (test-ob-core/dir-attach): Added unit test for new attach
feature
---
 doc/org-manual.org      |  6 ++++
 etc/ORG-NEWS            |  7 ++++
 lisp/ob-core.el         | 78 +++++++++++++++++++++++++++++------------
 lisp/org-attach.el      |  1 +
 testing/lisp/test-ob.el | 66 ++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 23 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 32a45f884..5f9b3353b 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -17541,6 +17541,12 @@ directory with {{{kbd(M-x cd RET DIRECTORY)}}}, and then not setting
 =dir=.  Under the surface, =dir= simply sets the value of the Emacs
 variable ~default-directory~.  Setting =mkdirp= header argument to
 a non-~nil~ value creates the directory, if necessary.
+Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
+set =dir= to the directory returned by ~(org-attach-dir)~, set =:mkdir
+yes=, and insert any file paths, as when using =:results file=, which
+are under the node's attachment directory using =attachment:= links
+instead of the usual =file:= links. Any returned path outside of the
+attachment directory will use =file:= links as per usual.
 
 For example, to save the plot file in the =Work/= folder of the home
 directory---notice tilde is expanded:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 35af94f92..0c88378d6 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -795,6 +795,13 @@ Finally, the closures are only evaluated if they're not overridden for
 a source block. This improves efficiency in cases where the result of
 a compute-expensive closure would otherwise be discarded.
 
+*** New special value ~'attach~ for src block =:dir= option
+
+Passing the symbol ~attach~ or string ="'attach"= (with quotes) to the =:dir=
+option of a src block is now equivalent to =:dir (org-attach-dir) :mkdir yes=
+and any file results with a path descended from the attachment directory will
+use =attachment:= style links instead of the standard =file:= link type.
+
 ** Miscellaneous
 *** =org-bibtex= includes =doi= and =url= entries when exporting to BiBTeX
 =doi= and =url= entries have been made optional for some publication
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 09d6adfe0..34002ac3e 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -801,7 +801,8 @@ block."
 		    (let ((*this* (if (not file) result
 				    (org-babel-result-to-file
 				     file
-				     (org-babel--file-desc params result)))))
+				     (org-babel--file-desc params result)
+                                     'attachment))))
 		      (setq result (org-babel-ref-resolve post))
 		      (when file
 			(setq result-params (remove "file" result-params))))))
@@ -2298,11 +2299,14 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result
+                 (org-babel-result-to-file
+		  result
+		  (org-babel--file-desc (nth 2 info) result)
+                  'attachment))))
 	((listp result))
 	(t (setq result (format "%S" result))))
+
   (if (and result-params (member "silent" result-params))
       (progn (message (replace-regexp-in-string "%" "%%" (format "%S" result)))
 	     result)
@@ -2605,27 +2609,43 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			      (expand-file-name
-			       (file-name-directory
-			        (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (base-directory (file-name-directory base-file-name))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        base-directory)))))
+           (request-attachment (eq type 'attachment))
+           (in-attach-dir (and request-attachment
+                               (string-prefix-p
+                                default-directory
+                                result-file-name))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ((and 'attachment (guard in-attach-dir)) "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
@@ -2756,10 +2776,22 @@ parameters when merging lists."
 	   (setq exports (funcall merge
 				  exports-exclusive-groups
 				  exports
-				  (split-string
+                                  (split-string
                                    (cond ((and value (functionp value)) (funcall value))
                                          (value value)
                                          (t ""))))))
+          ((or '(:dir . attach) '(:dir . "'attach"))
+           (unless (org-attach-dir nil t)
+             (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)))))
+           (setq params (append
+                         `((:dir . ,(org-attach-dir nil t))
+                           (:mkdirp . "yes"))
+                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))
 	  ;; Regular keywords: any value overwrites the previous one.
 	  (_ (setq params (cons pair (assq-delete-all (car pair) params)))))))
     ;; Handle `:var' and clear out colnames and rownames for replaced
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index 5ee2b84b2..6a061b2b3 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -324,6 +324,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (command-execute command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
index aa05f87a3..0c3783361 100644
--- a/testing/lisp/test-ob.el
+++ b/testing/lisp/test-ob.el
@@ -1770,6 +1770,72 @@ nil
                 (file-modes "t.sh")
               (delete-file "t.sh"))))))
 
+(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\")
+\"test.txt\"
+#+end_src"
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* String
+<point>#+begin_src elisp :dir \"'attach\" :results file
+(f-write-text \"attachment testing\" 'utf-8 \"test.txt\")
+\"test.txt\"
+#+end_src"
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* Existing ID
+<point>#+begin_src elisp :dir 'attach :results file
+(f-write-text \"attachment testing\" 'utf-8 \"test.txt\")
+\"test.txt\"
+#+end_src"
+    (org-id-get-create)
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* Existing DIR property
+:PROPERTIES:
+:DIR:      custom-attach-dir
+:END:
+
+<point>#+begin_src elisp :dir 'attach :results file
+(f-write-text \"attachment testing\" 'utf-8 \"test.txt\")
+\"test.txt\"
+#+end_src"
+    (message "DIR: %s" (org-attach-dir t))
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  )
+
 (ert-deftest test-ob-core/dir-mkdirp ()
   "Test :mkdirp with :dir header combination."
   (should-not
-- 
2.33.0.windows.2

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

[Ihor Radchenko]

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

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 3083 bytes --]

Ah sorry about that. I'm on a windows laptop and didn't have make, so was
testing interactively and they were passing.
I cleaned that up and remove the f-* usage and verified under Ubuntu (on
WSL) that the new tests are passing. I was getting some failures with
unrelated tests, but also get those in master as well.

Unless I'm missing something it looks like test-ob-core/dir-mkdirp covers
most other :dir usage.

I've remove the ID creation altogether and throw an error if org-attach-dir
is nil at that point. The error directs the user to add :ID: or :DIR: and
we don't need to do any guessing on their behalf. Implicit ID creation
should likely live elsewhere anyway.


On Sat, Jun 11, 2022 at 5:49 AM Ihor Radchenko <yantar92@gmail.com> wrote:

> 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
>

[-- Attachment #1.2: Type: text/html, Size: 4034 bytes --]

[-- Attachment #2: org-src-block-results-attach-dir.patch --]
[-- Type: application/octet-stream, Size: 11222 bytes --]

From 18df18bf5c8251f900bcb06db0bdca786ae5d354 Mon Sep 17 00:00:00 2001
From: Ryan Scott <ryan@5pmCasual.com>
Date: Fri, 10 Jun 2022 00:01:37 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
 src block

* ob-core.el (org-babel-merge-params): Specifying the symbol 'attach`
or string "'attach" as the value of the `:dir' header now functions as
":dir (org-attach-dir nil t) :mkdirp t"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `(org-attach-dir)' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file'
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-merge-params').
* test-ob.el (test-ob-core/dir-attach): Added unit test for new attach
feature
---
 doc/org-manual.org      |  6 ++++
 etc/ORG-NEWS            |  7 ++++
 lisp/ob-core.el         | 73 ++++++++++++++++++++++++++++-------------
 lisp/org-attach.el      |  1 +
 testing/lisp/test-ob.el | 66 +++++++++++++++++++++++++++++++++++++
 5 files changed, 130 insertions(+), 23 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 32a45f884..5f9b3353b 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -17541,6 +17541,12 @@ directory with {{{kbd(M-x cd RET DIRECTORY)}}}, and then not setting
 =dir=.  Under the surface, =dir= simply sets the value of the Emacs
 variable ~default-directory~.  Setting =mkdirp= header argument to
 a non-~nil~ value creates the directory, if necessary.
+Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
+set =dir= to the directory returned by ~(org-attach-dir)~, set =:mkdir
+yes=, and insert any file paths, as when using =:results file=, which
+are under the node's attachment directory using =attachment:= links
+instead of the usual =file:= links. Any returned path outside of the
+attachment directory will use =file:= links as per usual.
 
 For example, to save the plot file in the =Work/= folder of the home
 directory---notice tilde is expanded:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 35af94f92..0c88378d6 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -795,6 +795,13 @@ Finally, the closures are only evaluated if they're not overridden for
 a source block. This improves efficiency in cases where the result of
 a compute-expensive closure would otherwise be discarded.
 
+*** New special value ~'attach~ for src block =:dir= option
+
+Passing the symbol ~attach~ or string ="'attach"= (with quotes) to the =:dir=
+option of a src block is now equivalent to =:dir (org-attach-dir) :mkdir yes=
+and any file results with a path descended from the attachment directory will
+use =attachment:= style links instead of the standard =file:= link type.
+
 ** Miscellaneous
 *** =org-bibtex= includes =doi= and =url= entries when exporting to BiBTeX
 =doi= and =url= entries have been made optional for some publication
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 09d6adfe0..1b780ee78 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -801,7 +801,8 @@ block."
 		    (let ((*this* (if (not file) result
 				    (org-babel-result-to-file
 				     file
-				     (org-babel--file-desc params result)))))
+				     (org-babel--file-desc params result)
+                                     'attachment))))
 		      (setq result (org-babel-ref-resolve post))
 		      (when file
 			(setq result-params (remove "file" result-params))))))
@@ -2298,11 +2299,14 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result
+                 (org-babel-result-to-file
+		  result
+		  (org-babel--file-desc (nth 2 info) result)
+                  'attachment))))
 	((listp result))
 	(t (setq result (format "%S" result))))
+
   (if (and result-params (member "silent" result-params))
       (progn (message (replace-regexp-in-string "%" "%%" (format "%S" result)))
 	     result)
@@ -2605,27 +2609,43 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			      (expand-file-name
-			       (file-name-directory
-			        (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (base-directory (file-name-directory base-file-name))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        base-directory)))))
+           (request-attachment (eq type 'attachment))
+           (in-attach-dir (and request-attachment
+                               (string-prefix-p
+                                default-directory
+                                result-file-name))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ((and 'attachment (guard in-attach-dir)) "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
@@ -2756,10 +2776,17 @@ parameters when merging lists."
 	   (setq exports (funcall merge
 				  exports-exclusive-groups
 				  exports
-				  (split-string
+                                  (split-string
                                    (cond ((and value (functionp value)) (funcall value))
                                          (value value)
                                          (t ""))))))
+          ((or '(:dir . attach) '(:dir . "'attach"))
+           (unless (org-attach-dir nil t)
+             (error "No attachment directory for element (add :ID: or :DIR: property)"))
+           (setq params (append
+                         `((:dir . ,(org-attach-dir nil t))
+                           (:mkdirp . "yes"))
+                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))
 	  ;; Regular keywords: any value overwrites the previous one.
 	  (_ (setq params (cons pair (assq-delete-all (car pair) params)))))))
     ;; Handle `:var' and clear out colnames and rownames for replaced
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index 5ee2b84b2..6a061b2b3 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -324,6 +324,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (command-execute command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
index aa05f87a3..a62bd56bf 100644
--- a/testing/lisp/test-ob.el
+++ b/testing/lisp/test-ob.el
@@ -1770,6 +1770,72 @@ nil
                 (file-modes "t.sh")
               (delete-file "t.sh"))))))
 
+(ert-deftest test-ob-core/dir-attach ()
+  "Test :dir header using special 'attach value"
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach Symbol
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-id-get-create)
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach String
+<point>#+begin_src elisp :dir \"'attach\" :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-id-get-create)
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach with Existing DIR property
+:PROPERTIES:
+:DIR:      custom-attach-dir
+:END:
+
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (message "DIR: %s" (org-attach-dir t))
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should-error
+   (org-test-with-temp-text-in-file
+    "* 'attach with no ID or DIR
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]")))))
+
 (ert-deftest test-ob-core/dir-mkdirp ()
   "Test :mkdirp with :dir header combination."
   (should-not
-- 
2.33.0.windows.2

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

[Ihor Radchenko]

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

> Ah sorry about that. I'm on a windows laptop and didn't have make, so was
> testing interactively and they were passing.
> I cleaned that up and remove the f-* usage and verified under Ubuntu (on
> WSL) that the new tests are passing. I was getting some failures with
> unrelated tests, but also get those in master as well.

I am still getting test failures using Emacs 28.
All the tests are passing on main.

Example backtrace:

file-name-directory(nil)
  org-babel-result-to-file("/tmp/test.txt" nil attachment)
  org-babel-insert-result("/tmp/test.txt" ("replace" "value" "file" "g
  org-babel-execute-src-block()
  (progn (org-mode) (let ((point (string-match "<point>" inside-text))
  (unwind-protect (progn (org-mode) (let ((point (string-match "<point
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
  (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
  (let ((inside-text (if (stringp "\n<point>#+begin_src shell :results
  (let ((lexical-binding nil)) (let ((inside-text (if (stringp "\n<poi
  (lambda nil (let ((lexical-binding nil)) (let ((inside-text (if (str
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name test-ob/result-graphics-link-type-hea
  ert-run-or-rerun-test(#s(ert--stats :selector "test-ob" :tests ... :
  ert-run-tests("test-ob" #f(compiled-function (event-type &rest event
  ert-run-tests-batch("test-ob")
  ert-run-tests-batch-and-exit("test-ob")
  (let ((org-id-track-globally t) (org-test-selector (if org-test-sele
  org-test-run-batch-tests("test-ob")
  command-line-1(("--eval" "(setq vc-handled-backends nil org-startup-
  command-line()
  normal-top-level()
Test test-ob/result-graphics-link-type-header-argument condition:
    (wrong-type-argument stringp nil)
   FAILED  129/140  test-ob/result-graphics-link-type-header-argument (0.006189 sec)

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

Strange. I'll figure out a better setup for running the tests and get to
the bottom of that.
Thanks for the help.

On Tue, Jun 14, 2022, 00:10 Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> > Ah sorry about that. I'm on a windows laptop and didn't have make, so was
> > testing interactively and they were passing.
> > I cleaned that up and remove the f-* usage and verified under Ubuntu (on
> > WSL) that the new tests are passing. I was getting some failures with
> > unrelated tests, but also get those in master as well.
>
> I am still getting test failures using Emacs 28.
> All the tests are passing on main.
>
> Example backtrace:
>
> file-name-directory(nil)
>   org-babel-result-to-file("/tmp/test.txt" nil attachment)
>   org-babel-insert-result("/tmp/test.txt" ("replace" "value" "file" "g
>   org-babel-execute-src-block()
>   (progn (org-mode) (let ((point (string-match "<point>" inside-text))
>   (unwind-protect (progn (org-mode) (let ((point (string-match "<point
>   (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
>   (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
>   (let ((inside-text (if (stringp "\n<point>#+begin_src shell :results
>   (let ((lexical-binding nil)) (let ((inside-text (if (stringp "\n<poi
>   (lambda nil (let ((lexical-binding nil)) (let ((inside-text (if (str
>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>   ert-run-test(#s(ert-test :name test-ob/result-graphics-link-type-hea
>   ert-run-or-rerun-test(#s(ert--stats :selector "test-ob" :tests ... :
>   ert-run-tests("test-ob" #f(compiled-function (event-type &rest event
>   ert-run-tests-batch("test-ob")
>   ert-run-tests-batch-and-exit("test-ob")
>   (let ((org-id-track-globally t) (org-test-selector (if org-test-sele
>   org-test-run-batch-tests("test-ob")
>   command-line-1(("--eval" "(setq vc-handled-backends nil org-startup-
>   command-line()
>   normal-top-level()
> Test test-ob/result-graphics-link-type-header-argument condition:
>     (wrong-type-argument stringp nil)
>    FAILED  129/140  test-ob/result-graphics-link-type-header-argument
> (0.006189 sec)
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 2978 bytes --]

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

[Ryan Scott]

[-- Attachment #1.1: Type: text/plain, Size: 2845 bytes --]

I put together a clean setup in docker for running the tests and believe
I've gotten to the source of the problem.
However I've thought that a few times now.

This version of the patch has all tests passing.
The problem was in org-results-to-file and the attach dir detection. It
both had problems with an introduced assumption that the buffer-file-name
would be non-nil (causing several tests to fail) as well as being generally
overzealous in detecting file result paths as attachment links.


On Mon, Jun 13, 2022 at 10:55 PM Ryan Scott <ryan@vicarious-living.com>
wrote:

> Strange. I'll figure out a better setup for running the tests and get to
> the bottom of that.
> Thanks for the help.
>
> On Tue, Jun 14, 2022, 00:10 Ihor Radchenko <yantar92@gmail.com> wrote:
>
>> Ryan Scott <ryan@vicarious-living.com> writes:
>>
>> > Ah sorry about that. I'm on a windows laptop and didn't have make, so
>> was
>> > testing interactively and they were passing.
>> > I cleaned that up and remove the f-* usage and verified under Ubuntu (on
>> > WSL) that the new tests are passing. I was getting some failures with
>> > unrelated tests, but also get those in master as well.
>>
>> I am still getting test failures using Emacs 28.
>> All the tests are passing on main.
>>
>> Example backtrace:
>>
>> file-name-directory(nil)
>>   org-babel-result-to-file("/tmp/test.txt" nil attachment)
>>   org-babel-insert-result("/tmp/test.txt" ("replace" "value" "file" "g
>>   org-babel-execute-src-block()
>>   (progn (org-mode) (let ((point (string-match "<point>" inside-text))
>>   (unwind-protect (progn (org-mode) (let ((point (string-match "<point
>>   (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
>>   (let ((temp-buffer (generate-new-buffer " *temp*" t))) (save-current
>>   (let ((inside-text (if (stringp "\n<point>#+begin_src shell :results
>>   (let ((lexical-binding nil)) (let ((inside-text (if (stringp "\n<poi
>>   (lambda nil (let ((lexical-binding nil)) (let ((inside-text (if (str
>>   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
>>   ert-run-test(#s(ert-test :name test-ob/result-graphics-link-type-hea
>>   ert-run-or-rerun-test(#s(ert--stats :selector "test-ob" :tests ... :
>>   ert-run-tests("test-ob" #f(compiled-function (event-type &rest event
>>   ert-run-tests-batch("test-ob")
>>   ert-run-tests-batch-and-exit("test-ob")
>>   (let ((org-id-track-globally t) (org-test-selector (if org-test-sele
>>   org-test-run-batch-tests("test-ob")
>>   command-line-1(("--eval" "(setq vc-handled-backends nil org-startup-
>>   command-line()
>>   normal-top-level()
>> Test test-ob/result-graphics-link-type-header-argument condition:
>>     (wrong-type-argument stringp nil)
>>    FAILED  129/140  test-ob/result-graphics-link-type-header-argument
>> (0.006189 sec)
>>
>> Best,
>> Ihor
>>
>

[-- Attachment #1.2: Type: text/html, Size: 3905 bytes --]

[-- Attachment #2: org-src-block-results-attach-dir.patch --]
[-- Type: application/octet-stream, Size: 11527 bytes --]

From 757762aebe25229016b5dc854e894fd9427c0e3c Mon Sep 17 00:00:00 2001
From: Ryan Scott <rscott@roblox.com>
Date: Fri, 10 Jun 2022 00:01:37 -0700
Subject: [PATCH] ob-core.el/babel: Special handling for attachment links in
 src block

* ob-core.el (org-babel-merge-params): Specifying the symbol 'attach`
or string "'attach" as the value of the `:dir' header now functions as
":dir (org-attach-dir nil t) :mkdirp t"
(org-babel-result-to-file): Optional `TYPE' argument accepts symbol
'attachment` to fixup up paths under `(org-attach-dir)' and use the
link type "attachment:" when that is detected.
(org-babel-insert-result): Pass symbol `attachment' as `TYPE' to
`org-babel-result-to-file'
* org-attach.el (org-attach-dir): Added autoload header to simplify
dependencies necessary to support this feature (called in
`org-babel-merge-params').
* test-ob.el (test-ob-core/dir-attach): Added unit test for new attach
feature
---
 doc/org-manual.org      |  6 ++++
 etc/ORG-NEWS            |  7 ++++
 lisp/ob-core.el         | 79 +++++++++++++++++++++++++++++------------
 lisp/org-attach.el      |  1 +
 testing/lisp/test-ob.el | 66 ++++++++++++++++++++++++++++++++++
 5 files changed, 136 insertions(+), 23 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 32a45f884..5f9b3353b 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -17541,6 +17541,12 @@ directory with {{{kbd(M-x cd RET DIRECTORY)}}}, and then not setting
 =dir=.  Under the surface, =dir= simply sets the value of the Emacs
 variable ~default-directory~.  Setting =mkdirp= header argument to
 a non-~nil~ value creates the directory, if necessary.
+Setting =dir= to the symbol ~attach~ or the string ~"'attach"~ will
+set =dir= to the directory returned by ~(org-attach-dir)~, set =:mkdir
+yes=, and insert any file paths, as when using =:results file=, which
+are under the node's attachment directory using =attachment:= links
+instead of the usual =file:= links. Any returned path outside of the
+attachment directory will use =file:= links as per usual.
 
 For example, to save the plot file in the =Work/= folder of the home
 directory---notice tilde is expanded:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 35af94f92..0c88378d6 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -795,6 +795,13 @@ Finally, the closures are only evaluated if they're not overridden for
 a source block. This improves efficiency in cases where the result of
 a compute-expensive closure would otherwise be discarded.
 
+*** New special value ~'attach~ for src block =:dir= option
+
+Passing the symbol ~attach~ or string ="'attach"= (with quotes) to the =:dir=
+option of a src block is now equivalent to =:dir (org-attach-dir) :mkdir yes=
+and any file results with a path descended from the attachment directory will
+use =attachment:= style links instead of the standard =file:= link type.
+
 ** Miscellaneous
 *** =org-bibtex= includes =doi= and =url= entries when exporting to BiBTeX
 =doi= and =url= entries have been made optional for some publication
diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 09d6adfe0..c2446daa6 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -801,7 +801,8 @@ block."
 		    (let ((*this* (if (not file) result
 				    (org-babel-result-to-file
 				     file
-				     (org-babel--file-desc params result)))))
+				     (org-babel--file-desc params result)
+                                     'attachment))))
 		      (setq result (org-babel-ref-resolve post))
 		      (when file
 			(setq result-params (remove "file" result-params))))))
@@ -2298,11 +2299,14 @@ INFO may provide the values of these header arguments (in the
   (cond ((stringp result)
 	 (setq result (org-no-properties result))
 	 (when (member "file" result-params)
-	   (setq result (org-babel-result-to-file
-			 result
-			 (org-babel--file-desc (nth 2 info) result)))))
+	   (setq result
+                 (org-babel-result-to-file
+		  result
+		  (org-babel--file-desc (nth 2 info) result)
+                  'attachment))))
 	((listp result))
 	(t (setq result (format "%S" result))))
+
   (if (and result-params (member "silent" result-params))
       (progn (message (replace-regexp-in-string "%" "%%" (format "%S" result)))
 	     result)
@@ -2605,27 +2609,49 @@ in the buffer."
 		 (line-beginning-position 2))
 	     (point))))))
 
-(defun org-babel-result-to-file (result &optional description)
+(defun org-babel-result-to-file (result &optional description type)
   "Convert RESULT into an Org link with optional DESCRIPTION.
 If the `default-directory' is different from the containing
-file's directory then expand relative links."
+file's directory then expand relative links.
+
+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"
   (when (stringp result)
-    (let ((same-directory?
-	   (and (buffer-file-name (buffer-base-buffer))
-		(not (string= (expand-file-name default-directory)
-			      (expand-file-name
-			       (file-name-directory
-			        (buffer-file-name (buffer-base-buffer)))))))))
-      (format "[[file:%s]%s]"
-	      (if (and default-directory
-		       (buffer-file-name (buffer-base-buffer)) same-directory?)
-		  (if (eq org-link-file-path-type 'adaptive)
-		      (file-relative-name
-		       (expand-file-name result default-directory)
-		       (file-name-directory
-			(buffer-file-name (buffer-base-buffer))))
-		    (expand-file-name result default-directory))
-		result)
+    (let* ((result-file-name (expand-file-name result))
+           (base-file-name (buffer-file-name (buffer-base-buffer)))
+           (base-directory (and buffer-file-name
+                                (file-name-directory base-file-name)))
+           (same-directory?
+	    (and base-file-name
+	         (not (string= (expand-file-name default-directory)
+			       (expand-file-name
+			        base-directory)))))
+           (request-attachment (eq type 'attachment))
+           (attach-dir (let* ((default-directory base-directory)
+                              (dir (org-attach-dir nil t)))
+                         (when dir
+                           (expand-file-name dir))))
+           (in-attach-dir (and request-attachment
+                               attach-dir
+                               (string-prefix-p
+                                attach-dir
+                                result-file-name))))
+      (format "[[%s:%s]%s]"
+              (pcase type
+                ((and 'attachment (guard in-attach-dir)) "attachment")
+                (_ "file"))
+              (if (and request-attachment in-attach-dir)
+                  (file-relative-name result-file-name)
+	        (if (and default-directory
+		         base-file-name same-directory?)
+		    (if (eq org-link-file-path-type 'adaptive)
+		        (file-relative-name
+		         result-file-name
+                         (file-name-directory
+			  base-file-name))
+		      result-file-name)
+		  result))
 	      (if description (concat "[" description "]") "")))))
 
 (defun org-babel-examplify-region (beg end &optional results-switches inline)
@@ -2756,10 +2782,17 @@ parameters when merging lists."
 	   (setq exports (funcall merge
 				  exports-exclusive-groups
 				  exports
-				  (split-string
+                                  (split-string
                                    (cond ((and value (functionp value)) (funcall value))
                                          (value value)
                                          (t ""))))))
+          ((or '(:dir . attach) '(:dir . "'attach"))
+           (unless (org-attach-dir nil t)
+             (error "No attachment directory for element (add :ID: or :DIR: property)"))
+           (setq params (append
+                         `((:dir . ,(org-attach-dir nil t))
+                           (:mkdirp . "yes"))
+                         (assq-delete-all :dir (assq-delete-all :mkdir params)))))
 	  ;; Regular keywords: any value overwrites the previous one.
 	  (_ (setq params (cons pair (assq-delete-all (car pair) params)))))))
     ;; Handle `:var' and clear out colnames and rownames for replaced
diff --git a/lisp/org-attach.el b/lisp/org-attach.el
index 5ee2b84b2..6a061b2b3 100644
--- a/lisp/org-attach.el
+++ b/lisp/org-attach.el
@@ -324,6 +324,7 @@ Shows a list of commands and prompts for another key to execute a command."
 	    (command-execute command)
 	  (error "No such attachment command: %c" c))))))
 
+;;;###autoload
 (defun org-attach-dir (&optional create-if-not-exists-p no-fs-check)
   "Return the directory associated with the current outline node.
 First check for DIR property, then ID property.
diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
index aa05f87a3..a62bd56bf 100644
--- a/testing/lisp/test-ob.el
+++ b/testing/lisp/test-ob.el
@@ -1770,6 +1770,72 @@ nil
                 (file-modes "t.sh")
               (delete-file "t.sh"))))))
 
+(ert-deftest test-ob-core/dir-attach ()
+  "Test :dir header using special 'attach value"
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach Symbol
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-id-get-create)
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach String
+<point>#+begin_src elisp :dir \"'attach\" :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-id-get-create)
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should
+   (org-test-with-temp-text-in-file
+    "* 'attach with Existing DIR property
+:PROPERTIES:
+:DIR:      custom-attach-dir
+:END:
+
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (message "DIR: %s" (org-attach-dir t))
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]"))))
+  (should-error
+   (org-test-with-temp-text-in-file
+    "* 'attach with no ID or DIR
+<point>#+begin_src elisp :dir 'attach :results file
+(with-temp-file \"test.txt\" (insert \"attachment testing\n\"))
+\"test.txt\"
+#+end_src"
+    (org-babel-execute-src-block)
+    (goto-char (org-babel-where-is-src-block-result))
+    (forward-line)
+    (and
+     (file-exists-p (format "%s/test.txt" (org-attach-dir nil t)))
+     (string= (buffer-substring-no-properties (point) (line-end-position))
+              "[[attachment:test.txt]]")))))
+
 (ert-deftest test-ob-core/dir-mkdirp ()
   "Test :mkdirp with :dir header combination."
   (should-not
-- 
2.33.0.windows.2

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

[Ihor Radchenko]

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

> I put together a clean setup in docker for running the tests and believe
> I've gotten to the source of the problem.
> However I've thought that a few times now.

Would you be interested to share it? It might be helpful for other
people trying to develop on Windows.

> This version of the patch has all tests passing.
> The problem was in org-results-to-file and the attach dir detection. It
> both had problems with an introduced assumption that the buffer-file-name
> would be non-nil (causing several tests to fail) as well as being generally
> overzealous in detecting file result paths as attachment links.

Thanks!

Applied onto main via 226119124 with several amendments.

I have added some missing "." at the end of sentences, made sure that
all sentences are separated by doube "  " as required by our conventions
and dropped `quoting' from non-symbols.

Best,
Ihor

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

[Ryan Scott]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Awesome!
Thanks again for all the help. It's been a crash course in org internals
and the contribution process.
Well prepared for next time to go much smoother.

Cheers,
  -ryan

On Tue, Jun 14, 2022 at 6:47 AM Ihor Radchenko <yantar92@gmail.com> wrote:

> Ryan Scott <ryan@vicarious-living.com> writes:
>
> > I put together a clean setup in docker for running the tests and believe
> > I've gotten to the source of the problem.
> > However I've thought that a few times now.
>
> Would you be interested to share it? It might be helpful for other
> people trying to develop on Windows.
>
> > This version of the patch has all tests passing.
> > The problem was in org-results-to-file and the attach dir detection. It
> > both had problems with an introduced assumption that the buffer-file-name
> > would be non-nil (causing several tests to fail) as well as being
> generally
> > overzealous in detecting file result paths as attachment links.
>
> Thanks!
>
> Applied onto main via 226119124 with several amendments.
>
> I have added some missing "." at the end of sentences, made sure that
> all sentences are separated by doube "  " as required by our conventions
> and dropped `quoting' from non-symbols.
>
> Best,
> Ihor
>

[-- Attachment #2: Type: text/html, Size: 1769 bytes --]

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

[Ihor Radchenko]

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

>> > +             (if (or noninteractive (y-or-n-p (format "Create ID for
>> entry \"%s\"?"

One more thing I forgot.
Please, use `yes-or-no-p'. `y-or-n-p' is not a good idea - it is too
easy to accidently confirm by hitting space.

Best,
Ihor