thomasp85 / shinyFiles

A shiny extension for server side file access
196 stars 47 forks source link

Support to soflinks to directory in dir chooser #104

Closed raggaraluz closed 6 years ago

raggaraluz commented 6 years ago

Fix for #103

codecov-io commented 6 years ago

Codecov Report

Merging #104 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #104   +/-   ##
=====================================
  Coverage       0%     0%           
=====================================
  Files           4      4           
  Lines         444    444           
=====================================
  Misses        444    444
Impacted Files Coverage Δ
R/dirchoose.R 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 48761fc...8dc8f72. Read the comment docs.

vnijs commented 6 years ago

Thanks for the issue report and PR @raggaraluz! I think a slightly simpler solution would be to use file_exists rather than dir_exists. That way to wouldn't need link_exists and link_path. If you can (1) confirm that also works for you (on Windows?) and (2) update the PR I'd be happy to merge it.

vnijs commented 6 years ago

Thanks @raggaraluz!

raggaraluz commented 6 years ago

You are welcome! By the way, I've checked and works in windows as well

raggaraluz commented 6 years ago

Hi @vnijs again. I'm retesting it again and found a possible issue with the file_exists solution.

With the solution of file_exists, all softlinks are shown in the Directories path, even if they are pointing to a file instead of a directory. Therefore the user is allowed to select them, even if they are not proper directories.

The only way (that I've found) to show softlinks to directories and not sofltlinks to files is the solution with link_exists and link_path.

Should I revert the latest commit?

Example: test soflink is pointing to the DESCRIPTION file imagen

Using file_exists shows the test softlink imagen

However, using link_expand + dir_exists, it won't show (which is the expected behavior) imagen

In both cases, test2 softlink, which is pointing to a directory, is shown.

Sorry for the late testing, but just realized now about this case.

vnijs commented 6 years ago

Good catch @raggaraluz. I just created an issue (r-lib/fs#138).

The simplest work around requires an update to both dirchoose.R and filechoose.R. See https://github.com/thomasp85/shinyFiles/commit/752c01bc4ea1d66bfacdf6f1a7ffbf6703b050d7. Without the fix in filechoose.R links to files shown in the Content window would be shown as directories.

An important reason to use the r-lib/fs library is UTF-8 encoding of file and directory names. I checked and dir.exists doesn't cause any problems here. That said, please do check if everything works as expected on your machines

raggaraluz commented 6 years ago

Hi @vnijs, it works great now. Thanks a lot!

vnijs commented 6 years ago

Thanks for posting the issue and verifying that the fix actually works @raggaraluz