[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ob-R output file with graphics parameter
From: |
Jack Kamm |
Subject: |
Re: [PATCH] ob-R output file with graphics parameter |
Date: |
Sun, 27 Jun 2021 10:12:48 -0700 |
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.