More misc cleanup

docs/src/api.md

@@ -1,29 +1,35 @@
 # API
 
-All the standard methods for working with paths in base julia exist in the FilePathsBase.jl. The following describes the rough mapping of method names. Use `?` at the REPL to get the documentation and arguments as they may be different than the base implementations.
+To compare and contrast `FilePathsBase` with `Base.Filesystem` we provide tables for common operations.
+Use `?` at the REPL to get the documentation and arguments as they may be different than the base implementations.
 
-Base | FilePathsBase.jl
+## Operations
+
+A table of common operations with Filesystem and FilePathsBase.
+
+Filesystem | FilePathsBase.jl
 --- | ---
-"/home/user/docs" | `p"/home/user/docs"`
+"/home/user/docs" | p"/home/user/docs"
 N/A | Path()
-pwd() | pwd(::Type{<:AbstractPath}) (or cwd())
-homedir() | homedir(::Type{<:AbstractPath}) (or home())
+pwd() | pwd(SystemPath) or cwd()
+homedir() | homedir(SystemPath) or home()
 cd() | cd()
-joinpath() | joinpath(), join, /
+joinpath() | /
 basename() | basename()
 N/A | hasparent, parents, parent
 splitext | splitext
 N/A | filename
 N/A | extension
 N/A | extensions
 ispath | exists
-realpath | real
+realpath | canonicalize
 normpath | normalize
 abspath | absolute
 relpath | relative
 stat | stat
 lstat | lstat
 filemode | mode
+filesize | filesize
 mtime | modified
 ctime | created
 isdir | isdir
@@ -50,7 +56,7 @@ mv | mv
 download | download
 readdir | readdir
 N/A | readpath
-N/A | walkpath
+walkpath | walkpath
 rm | rm
 touch | touch
 tempname() | tempname(::Type{<:AbstractPath}) (or tmpname)
@@ -64,6 +70,53 @@ write | write
 @__DIR__ | @__PATH__
 @__FILE__ | @__FILEPATH__
 
+## Aliases
+
+A slightly reduced list of operations/aliases that will work with both strings and path types.
+The `Filesystem` and `FilePathsBase` columns indicate what type will be returned from each
+each library. As you'd expect, most return types match the input argument(s).
+
+Function Name | Filesystem | FilePathsBase
+--- | --- | ---
+cd | AbstractString | AbstractPath
+joinpath | AbstractString | AbstractPath
+basename | AbstractString | AbstractString
+splitext | (AbstractString, AbstractString) | (AbstractPath, AbstractString)
+ispath | Bool | Bool
+realpath | AbstractString | AbstractPath
+normpath | AbstractString | AbstractPath
+abspath | AbstractString | AbstractPath
+relpath | AbstractString | AbstractPath
+stat | StatStruct | FilePathsBase.Status
+lstat | StatStruct | FilePathsBase.Status
+filemode | UInt64 | FilePathsBase.Mode
+filesize | Int64 | Int64
+mtime | Float64 | Float64
+ctime | Float64 | Float64
+isdir | Bool | Bool
+isfile | Bool | Bool
+islink | Bool | Bool
+issocket | Bool | Bool
+isfifo | Bool | Bool
+ischardev | Bool | Bool
+isblockdev | Bool | Bool
+ismount | Bool | Bool
+isabspath | Bool | Bool
+expanduser | AbstractString | AbstractPath
+mkdir | AbstractString | AbstractPath
+mkpath | AbstractString | AbstractPath
+symlink | Nothing | Nothing
+cp | AbstractString | AbstractPath
+mv | AbstractString | AbstractPath
+download | AbstractString | AbstractPath
+readdir | AbstractString | AbstractString
+rm | Nothing | Nothing
+touch | AbstractString | AbstractPath
+chmod | AbstractString | AbstractPath
+chown | AbstractString | AbstractPath
+read(fp, T) | T | T
+write | Int64 | Int64
+
 
 ```@meta
 DocTestSetup = quote

docs/src/faq.md

@@ -20,10 +20,10 @@ Some cases where the path type distinction is useful include:
 
 See [design section](@ref design_header) for more details on the advantages of path types over strings.
 
-**Q. Why is `AbstractPath` not a subtype of `AbstractString`?
+**Q. Why is `AbstractPath` not a subtype of `AbstractString`?**
 
 A. Initially, we made `AbstractPath` a subtype of `AbstractString`, but falling back to string operations often didn't make sense (e.g., `ascii(::AbstractPath)`, `chomp(::AbstractPath)`, `match(::Regex, ::AbstractPath)`, `parse(::Type{Float64}, ::AbstractPath)`).
-Having a distinct path type results in fewer confusing error messages and more explicit code (via type conversions). [Link to issue/PR and blog post]
+Having a distinct path type results in fewer confusing error messages and more explicit code (via type conversions). See [issue #15](https://github.com/rofinn/FilePathsBase.jl/issues/15) for more info on why we dropped string subtyping.
 
 **Q. Why don't you concatenate paths with `*`?**
 

src/aliases.jl

@@ -19,3 +19,4 @@ Base.mtime(fp::AbstractPath) = datetime2unix(modified(fp))
 Base.normpath(fp::AbstractPath) = normalize(fp)
 Base.realpath(fp::AbstractPath) = canonicalize(fp)
 Base.relpath(fp::AbstractPath) = relative(fp)
+Base.relpath(fp::AbstractPath, src::AbstractPath) = relative(fp, src)

src/path.jl

@@ -3,25 +3,24 @@
 """
     Path() -> SystemPath
     Path(fp::Tuple) -> SystemPath
-    Path(fp::P) where P <: AbstractPath) -> P
     Path(fp::AbstractString) -> AbstractPath
-    Path(fp::P, segments::Tuple) -> P
+    Path(fp::P; overrides...) -> P
 
 Responsible for creating the appropriate platform specific path
 (e.g., [PosixPath](@ref) and [WindowsPath`](@ref) for Unix and
 Windows systems respectively)
-
-NOTE: `Path(::AbstractString)` can also work for custom paths if
-[ispathtype](@ref) is defined for that type.
 """
 function Path end
 
-Path(fp::AbstractPath) = fp
-Path(str::AbstractString) = parse(AbstractPath, str)
-function Path(fp::T, segments::Tuple{Vararg{String}}) where T <: AbstractPath
-    T((s === :segments ? segments : getfield(fp, s) for s in fieldnames(T))...)
+function Path(fp::T; overrides...) where T<:AbstractPath
+    override_fields = keys(overrides)
+    override_vals = values(overrides)
+
+    T((s in override_fields ? override_vals[s] : getfield(fp, s) for s in fieldnames(T))...)
 end
 
+Path(str::AbstractString) = parse(AbstractPath, str)
+
 """
     @p_str -> Path
 
@@ -224,11 +223,11 @@ function parents(fp::T) where {T <: AbstractPath}
     if hasparent(fp)
         # Iterate from 1:n-1 or 0:n-1 for relative and absolute paths respectively.
         # (i.e., include fp.root when applicable)
-        return [Path(fp, fp.segments[1:i]) for i in isrelative(fp):length(fp.segments) - 1]
+        return [Path(fp; segments=fp.segments[1:i]) for i in isrelative(fp):length(fp.segments) - 1]
     elseif fp.segments == tuple(".") || !isempty(fp.root)
         return [fp]
     else
-        return [Path(fp, tuple("."))]
+        return [Path(fp; segments=tuple("."))]
     end
 end
 
@@ -292,7 +291,7 @@ function join(prefix::AbstractPath, pieces::Union{AbstractPath, AbstractString}.
         end
     end
 
-    return Path(pre, tuple(segments...))
+    return Path(pre; segments=tuple(segments...))
 end
 
 function Base.splitext(fp::AbstractPath)
@@ -403,7 +402,7 @@ function normalize(fp::T) where {T <: AbstractPath}
         count += 1
     end
 
-    return Path(fp, tuple(fill("..", del)..., reverse(result)...))
+    return Path(fp; segments=tuple(fill("..", del)..., reverse(result)...))
 end
 
 """
@@ -460,7 +459,14 @@ function relative(fp::T, start::T=cwd()) where {T<:AbstractPath}
     else
         relpath_ = tuple(pathpart...)
     end
-    return isempty(relpath_) ? parse(T, curdir) : Path(fp, relpath_)
+
+    if isempty(relpath_)
+        return parse(T, curdir)
+    else
+        # Our assumption is that relative paths shouldn't have a root or drive.
+        # This seems to be consistent with Filesystem on Posix and Windows paths.
+        return Path(fp; segments=relpath_, drive="", root="")
+    end
 end
 
 """
@@ -530,7 +536,7 @@ Copy the file or directory from `src` to `dst`. An existing `dst` will only be o
 if `force=true`. If the path types support symlinks then `follow_symlinks=true` will
 copy the contents of the symlink to the destination.
 """
-function Base.cp(src::AbstractPath, dst::AbstractPath; force=false)
+function Base.cp(src::AbstractPath, dst::AbstractPath; force=false, follow_symlinks=false)
     if exists(dst)
         if force
             rm(dst; force=force, recursive=true)
@@ -549,6 +555,8 @@ function Base.cp(src::AbstractPath, dst::AbstractPath; force=false)
         end
     elseif isfile(src)
         write(dst, read(src))
+    elseif islink(src)
+        follow_symlinks ? symlink(readlink(src), dst) : write(dst, read(canonicalize(src)))
     else
         throw(ArgumentError("Source path is not a file or directory: $src"))
     end
@@ -565,6 +573,7 @@ Move the file or director from `src` to `dst`. An exist `dst` will only be overw
 function Base.mv(src::AbstractPath, dst::AbstractPath; force=false)
     cp(src, dst; force=force)
     rm(src; recursive=true)
+    return dst
 end
 
 """
@@ -634,7 +643,7 @@ function sync(f::Function, src::AbstractPath, dst::AbstractPath; delete=false, o
             index_pairs = collect(pairs(index))
             index_pairs = index_pairs[sortperm(last.(index_pairs))]
             for (seg, i) in index_pairs
-                cp(src_paths[i], Path(dst, tuple(dst.segments..., seg...)); force=true)
+                cp(src_paths[i], Path(dst, segments=tuple(dst.segments..., seg...)); force=true)
             end
         else
             cp(src, dst)

src/system.jl

@@ -189,7 +189,7 @@ function Base.cd(fn::Function, dir::SystemPath)
     end
 end
 
-function Base.mkdir(fp::SystemPath; mode=0o777, recursive=false, exist_ok=false)
+function Base.mkdir(fp::T; mode=0o777, recursive=false, exist_ok=false) where T<:SystemPath
     if exists(fp)
         !exist_ok && error("$fp already exists.")
     else
@@ -204,7 +204,7 @@ function Base.mkdir(fp::SystemPath; mode=0o777, recursive=false, exist_ok=false)
 			end
         end
 
-		mkdir(string(fp), mode=mode)
+		return parse(T, mkdir(string(fp), mode=mode))
     end
 end
 
@@ -227,13 +227,13 @@ end
 function Base.rm(fp::SystemPath; kwargs...)
     rm(string(fp); kwargs...)
 end
-Base.touch(fp::SystemPath) = touch(string(fp))
-function Base.mktemp(parent::SystemPath)
+Base.touch(fp::T) where {T<:SystemPath} = parse(T, touch(string(fp)))
+function Base.mktemp(parent::T) where T<:SystemPath
     fp, io = mktemp(string(parent))
-    return Path(fp), io
+    return parse(T, fp), io
 end
 
-Base.mktempdir(parent::SystemPath) = Path(mktempdir(string(parent)))
+Base.mktempdir(parent::T) where {T<:SystemPath}= parse(T, mktempdir(string(parent)))
 
 """
     chown(fp::SystemPath, user::AbstractString, group::AbstractString; recursive=false)
@@ -252,8 +252,12 @@ function Base.chown(fp::SystemPath, user::AbstractString, group::AbstractString;
     else
         error("chown is currently not supported on windows.")
     end
+
+    return fp
 end
 
+Base.chown(fp::T, args...) where {T<:SystemPath} = parse(T, chown(string(fp), args...))
+
 """
     chmod(fp::SystemPath, mode::Mode; recursive=false)
     chmod(fp::SystemPath, mode::Integer; recursive=false)
@@ -291,17 +295,17 @@ julia> mode(p"newfile")
 -rw-r--r--
 ```
 """
-function Base.chmod(fp::SystemPath, mode::Mode; recursive=false)
+function Base.chmod(fp::T, mode::Mode; recursive=false) where T<:SystemPath
     chmod_path = string(fp)
     chmod_mode = raw(mode)
 
     if isdir(fp) && recursive
         for p in readdir(fp)
-            chmod(chmod_path, chmod_mode; recursive=recursive)
+            parse(T, chmod(chmod_path, chmod_mode; recursive=recursive))
         end
     end
 
-    chmod(chmod_path, chmod_mode)
+    parse(T, chmod(chmod_path, chmod_mode))
 end
 
 function Base.chmod(fp::SystemPath, mode::Integer; recursive=false)
@@ -376,6 +380,15 @@ function Base.write(fp::SystemPath, x::Union{String, Vector{UInt8}}, mode="w")
 end
 
 Base.readdir(fp::SystemPath) = readdir(string(fp))
-Base.download(url::AbstractString, dest::SystemPath) = download(url, string(dest))
-Base.readlink(fp::SystemPath) = Path(readlink(string(fp)))
-canonicalize(fp::SystemPath) = Path(realpath(string(fp)))
+
+function Base.download(url::AbstractString, dest::T) where T<:SystemPath
+    return parse(T, download(url, string(dest)))
+end
+
+function Base.readlink(fp::T) where T<:SystemPath
+    return parse(T, readlink(string(fp)))
+end
+
+function canonicalize(fp::T) where T<:SystemPath
+    return parse(T, realpath(string(fp)))
+end

src/test.jl

@@ -255,7 +255,7 @@ module TestPaths
             _parents = parents(ps.qux)
             @test _parents[end] == ps.bar
             @test _parents[end - 1] == ps.root
-            @test _parents[1] == Path(ps.root, ())
+            @test _parents[1] == Path(ps.root; segments=())
 
             # More abstract path tests for edge cases when no parent exists
             @test hasparent(p"/foo")
@@ -376,6 +376,7 @@ module TestPaths
             @test normalize(ps.bar / ".." / "foo") == ps.foo
             @test normalize(ps.bar / ".") == ps.bar
             @test normpath(ps.bar / ".") == ps.bar
+
         end
     end
 

test/runtests.jl

@@ -1,6 +1,7 @@
 using FilePathsBase
 using FilePathsBase: /, join
 using Base.Filesystem
+using Dates: datetime2unix
 using JLSO
 using Test