Skip to content

Use proj_path_prep() on path from rstudioapi#485

Merged
jennybc merged 7 commits intomasterfrom
f-479-proj-path-prep
May 3, 2019
Merged

Use proj_path_prep() on path from rstudioapi#485
jennybc merged 7 commits intomasterfrom
f-479-proj-path-prep

Conversation

@jennybc
Copy link
Copy Markdown
Member

@jennybc jennybc commented Oct 27, 2018

Closes #479 Have usethis::proj_path_prep() use fs::path_abs(fs::path_expand()) instead of fs::path_real()

This causes 4 tests to fail. One was an easy and positive fix. One could be fixed (removed?), as it is literally testing for path realization. That is the first failure you see below.

The remaining 2 failures suggest a concrete reason why I used path_real() in the first place: rprojroot::find_root() returns a normalized path, in the sense of normalizePath(). That would not be particularly easy to fix / workaround.

This points out the difficulty of adopting a fs mindset when handing paths to/from other packages that use the base path handling toolkit. I have experienced similar pain with paths from RStudio via rstudioapi.

So if we want to do this, we will need to somehow put more wrapping around rprojroot::find_root(). Update: or, alternatively, more wrapping around any paths created by rstudioapi.

@jennybc jennybc requested a review from jimhester October 27, 2018 20:16
@jennybc
Copy link
Copy Markdown
Member Author

jennybc commented Oct 27, 2018

> test(filter = "proj")
Loading usethisSetting active project to '/Users/jenny/rrr/usethis'
Testing usethis| OK F W S | Context| 25 3     | projects [0.5 s]
───────────────────────────────────────────────────────────────────────────────────────────────────────────
test-proj.R:85: failure: proj_set() enforces proj path preparation policy
path_rel(proj_get(), a) not equal to "b/d".
1/1 mismatches
x[1]: "b2/d"
y[1]: "b/d"

test-proj.R:136: failure: with_project() runs code in temp proj, restores original proj
res[["active_usethis_proj"]] not identical to as.character(new_proj).
1/1 mismatches
x[1]: "/private/var/folders/yx/3p5dt4jj1019st0x90vhm9rr0000gn/T/RtmpI8hOSz/aaa922a2cabc31b"
y[1]: "/var/folders/yx/3p5dt4jj1019st0x90vhm9rr0000gn/T/RtmpI8hOSz/aaa922a2cabc31b"

test-proj.R:154: failure: local_project() activates proj til scope ends
res[["active_usethis_proj"]] not identical to as.character(new_proj).
1/1 mismatches
x[1]: "/private/var/folders/yx/3p5dt4jj1019st0x90vhm9rr0000gn/T/RtmpI8hOSz/aaa922a40963886"
y[1]: "/var/folders/yx/3p5dt4jj1019st0x90vhm9rr0000gn/T/RtmpI8hOSz/aaa922a40963886"
───────────────────────────────────────────────────────────────────────────────────────────────────────────

══ Results ════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 0.6 s

OK:       25
Failed:   3
Warnings: 0
Skipped:  0

@krlmlr
Copy link
Copy Markdown
Member

krlmlr commented Dec 1, 2018

I don't mind updating rprojroot to use fs, if that helps.

@hadley
Copy link
Copy Markdown
Member

hadley commented Dec 1, 2018

(@jennybc I updated this PR against master so make sure to pr_pull() it if you start working on it again. I did this to test my PR tooling, and indeed it found several bugs)

@jennybc
Copy link
Copy Markdown
Member Author

jennybc commented Dec 3, 2018

@jimhester Will you try this PR and see if it fixes your workflow on a Windows VM with a mapped network drive?

This version processes a path from rstudioapi as if it was an external user-provided path.

@jennybc
Copy link
Copy Markdown
Member Author

jennybc commented Dec 11, 2018

@jimhester Will you try this PR and see if it fixes your workflow on a Windows VM with a mapped network drive?

This version processes a path from rstudioapi as if it was an external user-provided path.

@jennybc
Copy link
Copy Markdown
Member Author

jennybc commented Mar 28, 2019

@jimhester We will release usethis soon. If you're still interested in this (I am!), will you try this PR and see if it fixes your workflow on a Windows VM with a mapped network drive?

This version processes a path from rstudioapi as if it was an external user-provided path.

@jennybc jennybc modified the milestone: Backlog Apr 5, 2019
Copy link
Copy Markdown
Member

@jimhester jimhester left a comment

Choose a reason for hiding this comment

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

So I finally tried this, extremely sorry for the delay!

It looks good and I verified it works with a mapped network drive on Windows.

Thanks!

@jennybc jennybc changed the title Change proj_path_prep() Use proj_path_prep() on path from rstudioapi May 3, 2019
@jennybc jennybc merged commit 9fa2026 into master May 3, 2019
@jennybc jennybc deleted the f-479-proj-path-prep branch May 10, 2019 19:41
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.

Have usethis::proj_path_prep() use fs::path_abs(fs::path_expand()) instead of fs::path_real()

4 participants