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

loadSupport(): fix global.R support, run global.R in appropriate dir #2831

Merged
merged 6 commits into from
Apr 22, 2020

Conversation

alandipert
Copy link
Contributor

@alandipert alandipert commented Apr 21, 2020

  • global.R support was broken as a result our changes to the appDir argument; this fixes it.
  • I added ye olde require(shiny) so shiny::loadSupport() works.
  • Ensures global.R is evaluated in the app directory instead of whichever directory loadSupport() was called in.
  • Replace use of on.exit()/setwd() with a withr::with_dir()
  • The order of files in R/ is sourced is now given by sort(..., method="radix") which aligns with R conventions
  • findEnclosingApp() now uses file.path.ci() internally so it now works on case-insensitive file systems (tested by mounting fat32 loopback disk on Linux)

These items came up preparing https://github.com/CRI-iAtlas/shiny-iatlas for module testing.

@alandipert alandipert requested review from schloerke and wch April 21, 2020 13:02
@wch
Copy link
Collaborator

wch commented Apr 21, 2020

I think the other files, in the R/ directory, need to be sourced in the app directory as well, though this should be checked first to make sure it's the correct behavior.

@alandipert
Copy link
Contributor Author

@wch I think you're right, I'll fix it up.

@alandipert
Copy link
Contributor Author

I did some experimentation with devtools::load_all(), which seems to me like the most analogous existing behavior. I found it to be true that sources in R/ are evaluated with the working dir initially set to the package dir. I also found that the directory is not reset between scripts. That is, one script may call setwd() and influence the working directory of subsequent scripts.

@alandipert
Copy link
Contributor Author

@wch apprised me of r-lib/roxygen2#1077 and we now sort files from R/ using sort(..., method = "radix") which is documented to use 'C' collation order.

@wch wch merged commit a432449 into master Apr 22, 2020
@wch wch deleted the alan-loadSupport-global-appdir branch April 22, 2020 13:54
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