Re: [PATCH] ob-R output file with graphics parameter

[Jack Kamm <jackkamm@gmail.com>]

Hi all,

> I obviously missing something. The above works for me without the
> patch. Unfortunately, I can't trace back the thread in order to
> understand the context.

I think this is a followup from this mail:

https://orgmode.org/list/87zgxc42qg.fsf@gmail.com/

wherein Jeremie states:

> The current patch have been tested for remote connections as well and
> AFAIK, nothing breaks.
> 
> But I'm afraid that the graphical output is broken and has long been
> even before the path. The test for graphical output is compromised and
> does not do the right test. I will suggest new ones.

So I think it's to do with graphical outputs in remote R sessions.

However, I tested Jeremie's example on latest org master and it also
worked fine for me, also on remote sessions.

Jeremie, could you clarify the issue this fixes?

> +(defun org-babel-output-link-path:R (params)
> +  "format org-link to file from PARAMS"
> +  
> +  (format "[[file:%s]]" (concat (cdr (assq :dir params))
> +                                 "/"
> +                        (cdr (assq :file params)))))

Rather than concat, I think it is better to use expand-file-name, and
also call file-name-as-directory on the directory component.

Stylistically, I don't think there should be a blank line between the
docstring and the code. The docstring should also start with a capital
letter and end with a period.

> +    (setq out-file-path  (concat
> +                          (replace-regexp-in-string "/ssh:.*?:" "" (cdr (assq :dir params)))
> +                                 "/"
> +                                      out-file))

Use org-babel-process-file-name instead of replace-regexp-in-string to
get the local path of the file.

> - Remove second the need for a second file parameter

This would indeed be nice.

The requirement for a second file parameter was added in Org 9.3 to
support the use case in this thread:

https://orgmode.org/list/3ac2f42a-8ff2-1464-fa36-451e2ef0eacd@pressure.to/

But this syntax is annoyingly verbose for ob-R users, and also broke
lots of ob-R examples prior to Org 9.3.

A simple fix might be to have the "graphics" flag implicitly add the
"file" flag as well. But we would need to first check that this doesn't
break other use cases.

> Subject: [PATCH 1/4] ob-R.el: Remove redundant argument to function

I think it would be better to squash these changes into a single commit.