Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work with absolute paths #41

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Work with absolute paths #41

wants to merge 11 commits into from

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 3, 2020

Work around rstudio/rmarkdown#1889.

R/downlit-md.R Outdated
@@ -30,6 +30,9 @@ downlit_md_path <- function(in_path, out_path, format = NULL) {
ast_path <- tempfile()
on.exit(unlink(ast_path))

in_path <- fs::path_abs(in_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason not to use normalizePath() here? I'd prefer to keep the dependencies lighter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file must exist, this is generally not true for out_path:

normalizePath("bogus")
#> Warning in normalizePath("bogus"): path[1]="bogus": No such file or directory
#> [1] "bogus"

Created on 2020-09-03 by the reprex package (v0.3.0)

Same with mustWork = FALSE sans warning.

We can work around if you think it's worth it, or copy code from fs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the other option is to require/document that the user supply absolute paths? That might be easier given that I expect relatively few people to be calling this function.

@krlmlr
Copy link
Member Author

krlmlr commented Sep 3, 2020

Works now. Do we need a test?

@hadley
Copy link
Member

hadley commented Sep 3, 2020

Yeah, a test would be useful I think.

@krlmlr
Copy link
Member Author

krlmlr commented Sep 4, 2020

Done.

@krlmlr krlmlr force-pushed the b-pandoc-convert-abs-path branch from 3206600 to 30a9ab7 Compare September 25, 2020 02:25
@krlmlr
Copy link
Member Author

krlmlr commented Sep 25, 2020

Resolved conflicts.

@krlmlr krlmlr requested a review from hadley May 24, 2021 07:48
@jennybc
Copy link
Member

jennybc commented Aug 17, 2022

I bumped into this issue today and, although rstudio/rmarkdown#1889 has been closed, I suspect it is not truly fixed. Anecdotally, it feels like rmarkdown::pandoc_convert(output =) only works below session temp dir.

I am having trouble using downlit_md_path(out_path = ) when out_path is not below session tempdir. I intend to follow-up with a better reprex and issue here or in rmarkdown, but in case I don't, I wanted to at least note that there still seem to be path problems as of 2022-08-16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants