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

Vignettes: adding info on relative path and encoding #31

Closed
wants to merge 1 commit into from
Closed

Vignettes: adding info on relative path and encoding #31

wants to merge 1 commit into from

Conversation

max-alletsee
Copy link

This expands the "Vignettes" vignette slightly to enhance the package documentation regarding specifities related to the working directory that has to be used when recording requests and requirements for the file encoding.

Both topics are currently only indirectly mentioned (working directory) or not mentioned (encoding) in the package documentation, but might be potentially helpful for other users.

Any feedback is welcome.

@@ -37,6 +39,8 @@ This turns off the request recording or mocking and cleans up the R session stat

Note that these code chunks have `include=FALSE`. This prevents them from being printed in the resulting Markdown, HTML, PDF, or whatever format document you produce. They're doing work behind the scenes, so you don't need them to be shown to your readers.

When your requests have been recorded and saved as a sub-folder that corresponds to your `vignette-name` in the vignettes directory, they will be used for all subsequent runs of the vignette. However, it's always assumed that the stored files are encoded in utf-8. If your files have a different encoding, you may need to change this manually to utf-8 for httptest to work properly.
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure about this. It sounds like you're describing a bug that should be addressed, not something to document.

FWIW looking at the source for save_response(), I don't see how you can get anything that isn't UTF-8 or plain ASCII, so I'd be interested to see a minimal reprex.

Copy link
Author

Choose a reason for hiding this comment

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

I definitely see your point. I have some example code where I reproducibly run into the exact same issue on a win10 computer. As I currently don't have access to linux/mac machines, i'm not entirely sure if this leads to the same problem. I'll try to verify that and come back to you.

@nealrichardson
Copy link
Owner

Circling back here, apologies for the delay.

On the first point about the vignette working directory, I wonder if something like #52 would help: if there is a "vignettes" subdirectory (perhaps with a vignette .Rmd matching the name given to start_vignette), step into that, else be relative to current working dir. What do you think?

On the encoding issues, we wrestled with Windows env stuff when porting the CI to GitHub Actions in #37, so there is now more "srsly, use UTF-8 everywhere" code. Can you retry your example and see if it is still a problem?

@nealrichardson
Copy link
Owner

I believe #55 should have resolved this. Please reopen if you find otherwise. Thanks again for your contribution here, even though we're not merging this PR.

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.

None yet

2 participants