[Top][All Lists]

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


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:


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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]