Allow orderly_shared_resource to be used with unnamed arguments. #136
Conversation
The previous behaviour was inconsistent with orderly_resource and often
redundant, as users commonly want to copy the resource using the same
name as the original file.
It is now possible to use `orderly_shared_resource("foo.txt")`, in which
case the source and destination name are assumed to be the same.
The old syntax is still available in case the user wants to change the
name while copying, and it is even possible to mix-and-match, renaming
some files but not others.
R/metadata.R
Outdated
| validate_shared_resource <- function(args, call) { | ||
| if (length(args) == 0) { | ||
| cli::cli_abort("'orderly_shared_resource' requires at least one argument", | ||
| call = call) | ||
| } | ||
| assert_named(args, unique = TRUE, call = environment()) | ||
|
|
There was a problem hiding this comment.
Can you see if you can harmonise this through validate_file_from_to (in outpack_misc.R) which contains most of this logic? I think it should just drop in directly?
There was a problem hiding this comment.
Aha yes. I hadn't noticed that function. Switched to using it.
We now get string interpolation on the filenames "for free". I'm not sure whether it makes much sense to support it. I could disable it easily by setting envir = rlang::empty_env() in the call to validate_file_from_to.
I'm even sure the interpolation is that useful in orderly_dependency / orderly_copy_files. As far as I can tell, it only expands the names, not the values. This means orderly_dependency(..., files = "${foo}") wouldn't work, because it would use the literal ${foo} as the source file name... We could fix that behaviour, or we could remove that feature entirely (is there any benefit in us doing the interpolation? Why can't the caller use sprintf?)
| "All elements of 'arguments to 'orderly_shared_resource'' must be strings") | ||
| expect_error( | ||
| orderly_shared_resource(a = 1, TRUE, "str"), | ||
| "All elements of 'arguments to 'orderly_shared_resource'' must be strings") |
There was a problem hiding this comment.
These messages are pretty terrible, but I couldn't find a way to deparse the ... reliably so I've just used a placeholder instead.
I'd almost be tempted to change orderly_shared_resource to accept a single files argument instead, making it consistent with all the other functions, including orderly_dependency, orderly_copy_files and orderly_resource, and would make the deparsing easy, but of course that's a breaking change.
The previous behaviour was inconsistent with orderly_resource and often redundant, as users commonly want to copy the resource using the same name as the original file.
It is now possible to use
orderly_shared_resource("foo.txt"), in which case the source and destination name are assumed to be the same.The old syntax is still available in case the user wants to change the name while copying, and it is even possible to mix-and-match, renaming some files but not others.