Skip to content

Work with absolute paths#41

Open
krlmlr wants to merge 11 commits into
mainfrom
b-pandoc-convert-abs-path
Open

Work with absolute paths#41
krlmlr wants to merge 11 commits into
mainfrom
b-pandoc-convert-abs-path

Conversation

@krlmlr

@krlmlr krlmlr commented Sep 3, 2020

Copy link
Copy Markdown
Member

Work around rstudio/rmarkdown#1889.

Comment thread R/downlit-md.R Outdated
ast_path <- tempfile()
on.exit(unlink(ast_path))

in_path <- fs::path_abs(in_path)

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

krlmlr commented Sep 3, 2020

Copy link
Copy Markdown
Member Author

Works now. Do we need a test?

@hadley

hadley commented Sep 3, 2020

Copy link
Copy Markdown
Member

Yeah, a test would be useful I think.

@krlmlr

krlmlr commented Sep 4, 2020

Copy link
Copy Markdown
Member Author

Done.

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

krlmlr commented Sep 25, 2020

Copy link
Copy Markdown
Member Author

Resolved conflicts.

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

jennybc commented Aug 17, 2022

Copy link
Copy Markdown
Member

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