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

Absolute child is error prone #88

Open
ronanM opened this issue Oct 15, 2016 · 8 comments
Open

Absolute child is error prone #88

ronanM opened this issue Oct 15, 2016 · 8 comments

Comments

@ronanM
Copy link
Contributor

ronanM commented Oct 15, 2016

"append" an absolute child is error prone, and not consistent with Bash.

import better.files._
import better.files.File.home

val bashrc = home / ".bashrc"    
// --> /home/user/.bashrc

val bashrcBak = file"/tmp/aBackupDir" / (bashrc.pathAsString + ".bak")
// --> /home/user/.bashrc.bak
// Not /tmp/aBackupDir//home/user/.bashrc.bak
// Not /tmp/aBackupDir/home/user/.bashrc.bak

With Bash : /tmp/aBackupDir//home/user/.bashrc.bak

$ cat "/tmp/aBackupDir"/"/home/user/.bashrc.bak"
cat: /tmp/aBackupDir//home/user/.bashrc.bak: No such file or directory
@pathikrit pathikrit added the bug label Nov 10, 2016
@pathikrit
Copy link
Owner

I think we need to implement a typed path for relative and absolute paths to prevent stuff like this

@ronanM
Copy link
Contributor Author

ronanM commented Nov 10, 2016

@pathikrit
Copy link
Owner

pathikrit commented Nov 13, 2016

Yup, @lihaoyi has the right idea in ammonite about typing paths. Paths would also address some of the fuzziness regarding files that actually exist on the file system vs. doing operations on paths (see my comment here: #89 (comment) where I don't have a good answer for if File.getExtension should do I/O or just string operation).

See also: https://github.com/slamdata/scala-pathy

@jaa127
Copy link
Contributor

jaa127 commented Feb 12, 2017

Hello better-files!

It would be nice and handy to have kind of "getPathWithAnchor" functionality, with following semantics (which is really similar to java.nio.file.Path.resolve, with one twist - it supports either dir or file as reference):

/**
 * Get absolute path with help of reference anchor
 * Anchor is used as reference in case that path
 * is not absolute and it could be path to directory or
 * file. If it's file, then file's parent dir is used
 * as an anchor.
 *
 * If type of anchor is unknown, then it is treated as
 * file. One reason for unknown type (dir or file) could be  
 * if path is not a real, existing path on filesystem.
 *
 * @param pathStr relative or absolute path as string
 * @param anchor  reference path to be used in case pathStr
 *                is not absolute
 * @return absolute and normalized path to pathStr
 */

The main use case for this functionality is e.g. when you are dealing with configuration files
and you have a well establish path anchor (which could be file or directory) and you like to use that anchor conditionally when building new path (if other path is relative or absolute).

The main differences compared to java.nio.file.Path.resolve are:

  • Support for either file or directory as anchor
  • Result is absolute and normalized

Maybe this could be something like that with better-files

  def apply(anchor: Path, path: String): File

or even

  def apply(anchor: Path, path: String, fragments: String*): File

And usage would be something like:

val home = File("/User/johndoe/")
val f1 = File(home, "Documents/scala/io")
val f2 = File(home, "Documents", "scala", "io")

In my soap-box this would really handy. What do you think? If this makes sense, then I can provide PR for this feature.

P.S. There is one implementation of that here (I am an author of sn127, so MIT licensing some of that code / tests won't be problem):

https://github.com/sn127/utils/blob/b116036de96f7b66fba29117ce91168bf4323c45/fs/src/main/scala/fi/sn127/utils/fs/FileUtils.scala#L167

https://github.com/sn127/utils/blob/b116036de96f7b66fba29117ce91168bf4323c45/fs/src/test/scala/fi/sn127/utils/fs/FileUtilsTest.scala#L60

@pathikrit
Copy link
Owner

@jaa127 : Although your suggestion does not resolve the original bug this issue pertains to but it definitely makes sense and so please submit a PR.
Some guidelines:

  • In better-files we avoid any contact with Java as much as possible so I would say create the interface as def apply(anchor: File, path: String, fragments: String*): File instead of def apply(anchor: Path, path: String, fragments: String*): File?

  • Why do we need separate path and fragments? Can we have one argument for both? e.g. def apply(anchor: File, fragments: String*): File? If fragments is empty it simply returns the anchor?

@jvican
Copy link

jvican commented Mar 8, 2017

@jaa127 What you propose is really interesting, I would like to see it in better-files, especifically now that we're adding it as a Scala Platform module!

Are you thinking of contributing it? We would really welcome such a PR and we can help you out getting it done.

@jaa127
Copy link
Contributor

jaa127 commented Mar 8, 2017

@jvican Thanks! Fortunately @pathikrit has already merged that feature (PR #113), so it should/will be in v3.0.0.

@jvican
Copy link

jvican commented Mar 8, 2017

Aha, so you already did! @jaa127 😉

Cool to see this merged. We have some other issues that you may want to play around with -- we're currently trying to prepare for a release in the Scala Platform, the next community-maintained Scala library that will replace the current standard library in 2.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants