Use original path names in compiler outputs #198
Conversation
|
@sbearrows if you could take this as a starting point and push it further it would be great! You can use |
|
Okay definitely! |
|
@sbearrows this looks like it needs a news entry still, and do you think we should try to address #202 in this PR or separately? |
|
I was thinking I would do #202 in this one for simplicity. Thoughts? |
|
Sure, that is fine! |
| stop("Compilation failed.", call. = FALSE) | ||
| } | ||
|
|
||
|
|
||
| shared_lib <- file.path(dir, "src", paste0(tools::file_path_sans_ext(basename(file)), .Platform$dynlib.ext)) | ||
| r_path <- file.path(dir, "R", "cpp11.R") | ||
| brio::write_lines(r_functions, r_path) |
R/source.R
Outdated
|
|
||
| #change variable name to reflect this | ||
| file <- file.path(new_path, name) |
There was a problem hiding this comment.
lets name this something else, new_file_path???
R/source.R
Outdated
| name <- basename(file) | ||
| package <- tools::file_path_sans_ext(generate_cpp_name(file)) | ||
| } | ||
| orig_path <- normalizePath(dirname(file), winslash = "/") |
There was a problem hiding this comment.
| orig_path <- normalizePath(dirname(file), winslash = "/") | |
| orig_dir <- normalizePath(dirname(file), winslash = "/") |
R/source.R
Outdated
| error_messages <- res$stderr | ||
|
|
||
| # Substitute temporary file path with original file path | ||
| error_messages <- gsub(file.path(new_path, basename(file)), file.path(orig_path, basename(file)), error_messages, fixed = TRUE) |
There was a problem hiding this comment.
| error_messages <- gsub(file.path(new_path, basename(file)), file.path(orig_path, basename(file)), error_messages, fixed = TRUE) | |
| error_messages <- sub(file.path(new_path, basename(file)), file.path(orig_path, basename(file)), error_messages, fixed = TRUE) |
There was a problem hiding this comment.
filename <- basename(file)
new_path <- file.path(new_dir, filename)
orig_path <- file.path(orig_dir, filename)
error_messages <- sub(paste0(new_path, ": "), paste0(orig_path, ": "), error_messages, fixed = TRUE)
|
|
||
| # Substitute temporary file path with original file path | ||
| error_messages <- gsub(file.path(new_path, basename(file)), file.path(orig_path, basename(file)), error_messages, fixed = TRUE) | ||
| cat(error_messages) |
There was a problem hiding this comment.
| cat(error_messages) | |
| cat(error_messages, file = stderr()) |
tests/testthat/test-source.R
Outdated
| @@ -197,4 +210,12 @@ test_that("check_valid_attributes returns an error if one or more registers is i | |||
| } | |||
| ') | |||
| ) | |||
|
|
|||
jimhester
changed the title
|
The consequence of this is that errors are printed now twice with different base paths. First ones with I understand that this is a hack to account for some editors (per #194) but there are editors out there which do understand tmp paths and redirect automatically to the correct file location. So those Regardless of the editor I think it's cleaner and less confusing not to print |
|
While playing with it I think I know why the change was reverted. It's because the corner case of souring cpp11.cpp as per this test. I think that corner case can just be addressed by changing only cpp11.cpp input file when detected. I am adding one more commit in #251 with tests and some clarification inline docs to avoid breaking this again. |
This is a first draft at fixing #194.
It works for me locally on macOS. We would need to do some testing on Windows and maybe think about improving the search terms to be more specific. (include the trailing ':') in the error maybe?
Fixes #194
Fixes #202