Skip to content

Fail fast and informatively for nonexistent file #261

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

Merged
merged 5 commits into from
Feb 7, 2022

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Feb 4, 2022

If I call cpp_source() with a nonexistent filepath, interactively I see:

> devtools::load_all(".")
ℹ Loading cpp11
> cpp_source("asdfuiop.cpp")
Error in startsWith(decorations$decoration, "cpp11::") : 
  non-character object(s)

I'm not sure why the first snapshot in this PR shows a different (but also non informative) error. But, in any case, it seems better to check early for the existence of file and to fail in a more specific way, if file does not exist.

@@ -67,6 +67,9 @@
#' @export
cpp_source <- function(file, code = NULL, env = parent.frame(), clean = TRUE, quiet = TRUE, cxx_std = Sys.getenv("CXX_STD", "CXX11"), dir = tempfile()) {
stop_unless_installed(c("brio", "callr", "cli", "decor", "desc", "glue", "tibble", "vctrs"))
if (!missing(file) && !file.exists(file)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

!missing(file) seems a bit counter to our "house style". I wonder if the signature should become file = NULL, to indicate file is not required (at least, if code is provided)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be informative about the name/path of the file that does not exist ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking that too. I've added the non-existent file path to the error now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Perhaps ultimately this is something fs:: can help with ?

@romainfrancois romainfrancois merged commit 322e5ee into main Feb 7, 2022
@jennybc jennybc deleted the fail-fast-for-nonexistent-file branch March 14, 2023 03:44
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.

2 participants