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

Support directory traversal to find package root #616

Merged
merged 7 commits into from Oct 31, 2014

Conversation

Projects
None yet
2 participants
@robertzk
Contributor

robertzk commented Oct 10, 2014

In response to the following issue, I suggest support for referencing packages using load_all("some/package/R") or load_all("some/package/inst/tests") (and document, test, etc.) by traversing up the directory structure for the DESCRIPTION file -- not having this can get annoying.

I have modified check_dir in package.R and added a package_root helper function that returns the absolute path of such a parent directory, or NA_character_ otherwise. For example, package_root('some/package/inst/tests') is '/absolute/path/to/some/package'.

@robertzk

This comment has been minimized.

Contributor

robertzk commented Oct 10, 2014

@gaborcsardi Still working on Windows support, but is this better? Thanks for the feedback.

@robertzk

This comment has been minimized.

Contributor

robertzk commented Oct 10, 2014

@gaborcsardi Feel free to devtools::install_github('robertzk/devtools@load_from_anywhere') to try it out yourself (yes, I realize this is a little recursive, but I promise my PR doesn't break install_github to change back to hadley's version).

@robertzk

This comment has been minimized.

Contributor

robertzk commented Oct 15, 2014

bump - per our convo at R day, will refactor to use standard testing

@hadley

View changes

R/package.r Outdated
@@ -34,6 +35,19 @@ check_dir <- function(x) {
x
}
package_root <- function(path) {
if (is.package(path)) return(path$path)
else if (!is.character(path)) return(NULL)

This comment has been minimized.

@hadley

hadley Oct 15, 2014

Member

Shouldn't that be an error?

@hadley

View changes

R/package.r Outdated
has_description <- function(path) file.exists(file.path(path, 'DESCRIPTION'))
path <- suppressWarnings(normalizePath(path))
while (!has_description(path) && path != '/' && path != '.') path <- dirname(path)
if (path == '/' || path == '.') NULL

This comment has been minimized.

@hadley

hadley Oct 15, 2014

Member

How about something like is_root <- function(x) identical(x, dirname(x)) ?

This comment has been minimized.

@robertzk

robertzk Oct 15, 2014

Contributor

Very nice

@robertzk

This comment has been minimized.

Contributor

robertzk commented Oct 15, 2014

Better?

@hadley

View changes

R/package.r Outdated
stopifnot(is.character(path))
has_description <- function(path) file.exists(file.path(path, 'DESCRIPTION'))
path <- suppressWarnings(normalizePath(path))

This comment has been minimized.

@hadley

hadley Oct 24, 2014

Member

Maybe just normalizePath(path, mustWork = FALSE)?

@hadley

View changes

R/package.r Outdated
path <- suppressWarnings(normalizePath(path))
while (!has_description(path) && !is_root(path)) path <- dirname(path)
if (is_root(path)) NULL

This comment has been minimized.

@hadley

hadley Oct 24, 2014

Member

Can you please use {} here? (http://r-pkgs.had.co.nz/style.html)

@hadley

View changes

tests/testthat/test-load-hooks.r Outdated
@@ -143,3 +143,13 @@ test_that("onUnload", {
# Clean up
rm(".__testLoadHooks__", envir = .GlobalEnv)
})
test_that("it can load from outside of package root", {

This comment has been minimized.

@hadley

hadley Oct 24, 2014

Member

I'd put this in a new test file test-package.R

@robertzk

This comment has been minimized.

Contributor

robertzk commented Oct 24, 2014

Thanks for the comments! I think I incorporated every suggestion.

hadley added a commit that referenced this pull request Oct 31, 2014

Merge pull request #616 from robertzk/load_from_anywhere
Support directory traversal to find package root

@hadley hadley merged commit 2dcdfdc into r-lib:master Oct 31, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Oct 31, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment