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

use_test() without arguments fails with RStudio project under ~ on Windows #901

Closed
mikmart opened this issue Sep 28, 2019 · 11 comments · Fixed by #903
Closed

use_test() without arguments fails with RStudio project under ~ on Windows #901

mikmart opened this issue Sep 28, 2019 · 11 comments · Fixed by #903

Comments

@mikmart
Copy link

mikmart commented Sep 28, 2019

File path checking seems to go wrong when use_test() tries to get the path to the active file in RStudio in a project under ~ on Windows:

usethis::use_r("foo.R")
# ✔ Setting active project to 'C:/Users/mikko/Documents/R/dev/foo'
# ● Modify 'R/foo.R'
usethis::use_test()
# ✔ Adding 'testthat' to Suggests field in DESCRIPTION
# ✔ Creating 'tests/testthat/'
# ✔ Writing 'tests/testthat.R'
# ● Call `use_test()` to initialize a basic test file and open it for editing.
# Error: Open file must be in the 'R/' directory of the active package.
# * Actual path: 'Users/mikko/R/dev/foo/R/foo.R'
rstudioapi::getSourceEditorContext()$path
# [1] "~/R/dev/foo/R/foo.R"
usethis:::proj_path_prep(rstudioapi::getSourceEditorContext()$path)
# C:/Users/mikko/Documents/R/dev/foo/Users/mikko/R/dev/foo/R/foo.R

Related r-lib/fs#221. Maybe related #485?

@jennybc
Copy link
Member

jennybc commented Sep 29, 2019

Can I see full session info? And the output of proj_get(); proj_sitrep()?

usethis is stuck between a rock and a hard place given general file path misery on Windows, what rprojroot wants to do, what RStudio IDE wants to do, and what fs does (all of which are in some tension with each other).

A general design principle of usethis is to align with fs in terms of our understanding of ~ and the user's home directory.

https://usethis.r-lib.org/news/index.html#file-system

@cderv
Copy link
Contributor

cderv commented Sep 30, 2019

As a windows user, I can confirm the error. I don't recall that this happened before and I am a usethis user, so there may be a change that broke this (here or in fs).

About the situation, usethis see the project correctly

> usethis::proj_get()
C:/Users/chris/Documents/testpackage
> usethis::proj_sitrep()
*   working_directory: 'C:/Users/chris/Documents/testpackage'
* active_usethis_proj: 'C:/Users/chris/Documents/testpackage'
* active_rstudio_proj: 'C:/Users/chris/Documents/testpackage'

It is usethis:::get_active_r_file() which throws the error, and more precisely, it is usethis:::proj_path_prep that creates the wrong path.

> rstudioapi::getSourceEditorContext()$path
[1] "~/testpackage/R/foo.R"
> usethis:::proj_path_prep(rstudioapi::getSourceEditorContext()$path)
C:/Users/chris/Documents/testpackage/Users/chris/testpackage/R/foo.R

We can obviously see that there is something wrong in the last resulting path.
This function calls fs::path_real directly. I think @mikmart is right, and this comes from fs way of dealing with ~ in path_real.

  • usethis calls rstudioapi that returns a path using ~, with the R meaning.
  • usethis calls fs to get the full path using path_real, and it is here that the resulting path gets mixed up.

I think this should be handle in fs. I still don't understand the result of path_real

file.create(temp_file <- tempfile(tmpdir = "~"))
#> [1] TRUE
temp_file
#> [1] "~\\file8e68207943b3"
fs::path_real(temp_file)
#> C:/Users/chris/AppData/Local/Temp/Rtmp2pqIX4/reprex8058645525b2/Users/chris/file8e68207943b3
fs::path_expand(temp_file)
#> C:/Users/chris/file8e68207943b3
fs::path_expand_r(temp_file)
#> C:/Users/chris/Documents/file8e68207943b3
path.expand(temp_file)
#> [1] "C:/Users/chris/Documents\\file8e68207943b3"
normalizePath(temp_file)
#> [1] "C:\\Users\\chris\\Documents\\file8e68207943b3"
unlink(temp_file)

Created on 2019-09-30 by the reprex package (v0.3.0)

@mikmart
Copy link
Author

mikmart commented Sep 30, 2019

@jennybc here's the additional info in a fresh session from my side, too:

usethis::use_test()
#> ✔ Setting active project to 'C:/Users/mikko/Documents/R/dev/foo'
#> Error: Open file must be in the 'R/' directory of the active package.
#> * Actual path: 'Users/mikko/R/dev/foo/R/foo.R'
usethis::proj_get()
#> C:/Users/mikko/Documents/R/dev/foo
usethis::proj_sitrep()
#> *   working_directory: 'C:/Users/mikko/Documents/R/dev/foo'
#> * active_usethis_proj: 'C:/Users/mikko/Documents/R/dev/foo'
#> * active_rstudio_proj: 'C:/Users/mikko/Documents/R/dev/foo'
sessioninfo::session_info()
#> - Session info ------------------------------------------------------------
#>   setting  value                       
#> version  R version 3.6.1 (2019-07-05)
#> os       Windows 10 x64              
#> system   x86_64, mingw32             
#> ui       RStudio                     
#> language (EN)                        
#> collate  English_United States.1252  
#> ctype    English_United States.1252  
#> tz       Europe/Helsinki             
#> date     2019-09-30                  
#> 
#> - Packages ----------------------------------------------------------------
#>   package     * version    date       lib source                        
#> assertthat    0.2.1      2019-03-21 [1] CRAN (R 3.6.0)                
#> backports     1.1.4      2019-04-10 [1] CRAN (R 3.6.0)                
#> cli           1.1.0      2019-03-19 [1] CRAN (R 3.6.0)                
#> clisymbols    1.2.0      2017-05-21 [1] CRAN (R 3.6.0)                
#> crayon        1.3.4      2017-09-16 [1] CRAN (R 3.6.0)                
#> fs            1.3.1.9000 2019-09-28 [1] Github (r-lib/fs@380685c)     
#> glue          1.3.1      2019-03-12 [1] CRAN (R 3.6.0)                
#> magrittr      1.5        2014-11-22 [1] CRAN (R 3.6.0)                
#> purrr         0.3.2      2019-03-15 [1] CRAN (R 3.6.0)                
#> Rcpp          1.0.2      2019-07-25 [1] CRAN (R 3.6.1)                
#> rlang         0.4.0      2019-06-25 [1] CRAN (R 3.6.0)                
#> rprojroot     1.3-2      2018-01-03 [1] CRAN (R 3.6.0)                
#> rstudioapi    0.10       2019-03-19 [1] CRAN (R 3.6.0)                
#> sessioninfo   1.1.1      2018-11-05 [1] CRAN (R 3.6.0)                
#> usethis       1.5.1.9000 2019-09-28 [1] Github (r-lib/usethis@a2342b8)
#> withr         2.1.2      2018-03-15 [1] CRAN (R 3.6.0)                
#> 
#> [1] C:/Users/mikko/Documents/R/win-library/3.6
#> [2] C:/Program Files/R/R-3.6.1/library

@mikmart
Copy link
Author

mikmart commented Sep 30, 2019

I tried playing around with setting the R_USER envvar to align with the usethis and fs understanding of ~, but that didn't seem to help here. I also found it a bit confusing with e.g. file.edit() which I discovered is shimmed by the RStudio IDE, and doesn't respect R_USER for tilde expansion.

I wonder, is there any way to tell RStudio how ~ should be expanded?

@jennybc
Copy link
Member

jennybc commented Sep 30, 2019

I suspect this problem arose when I merged #485.

If true, then, without further pre-processing of RStudio-provided paths, we seem to be choosing between having ~/ interpreted correctly or being able to work with paths on a mapped network drive 😔

cc @jimhester

@cderv
Copy link
Contributor

cderv commented Sep 30, 2019

If rstudioapi::getSourceEditorContext() returns full path instead of one begining with ~ on windows, would it help ? I don't know what it would mean for rstudioapi and if feasible 🤔 you may already have think of that 😕

@jennybc
Copy link
Member

jennybc commented Sep 30, 2019

Yeah that would definitely help. And I feel like I've even discussed this with the IDE team before. But I should probably just work around this with a shim in usethis.

@jennybc
Copy link
Member

jennybc commented Sep 30, 2019

@mikmart and @cderv Does the PR fix it for you? It seems to do so for me, on my Windows VM.

@jimhester Could you confirm that the fix doesn't re-break things with a mapped drive on Windows?

@mikmart
Copy link
Author

mikmart commented Sep 30, 2019

Yup, it's working for me now, too! 👍

@jennybc
Copy link
Member

jennybc commented Oct 1, 2019

This is the existing RStudio IDE issue I was thinking about. But this PR is a workaround implemented here.

rstudio/rstudioapi#104

@mikmart
Copy link
Author

mikmart commented Oct 1, 2019

Thanks for the quick fix! 🙏

-- and also the link to the rstudioapi issue: that made me realize why my attempts at setting R_USER were ineffective with the IDE. 🤦‍♂ I was trying to set it in .Renviron, so it was too late for RStudio by then. Setting it on the system level changed the meaning of ~ in RStudio, too.

jennybc added a commit that referenced this issue Oct 1, 2019
* Make get_active_r_file() more robust to `~/` issues on Windows

Closes #901

* Simplify

* Add NEWS bullet
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 a pull request may close this issue.

3 participants