Skip to content

Commit

Permalink
Fix #31368: joinpath works on collections of paths (#38263)
Browse files Browse the repository at this point in the history
* Fix #31368: joinpath works on collections of paths

* Update base/path.jl

Co-authored-by: Jameson Nash <[email protected]>

* Apply suggestions from code review

* Fixup

Co-authored-by: Mustafa M <[email protected]>
Co-authored-by: Jameson Nash <[email protected]>
  • Loading branch information
3 people authored Apr 22, 2021
1 parent 15a44e9 commit fc02458
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 9 deletions.
37 changes: 28 additions & 9 deletions base/path.jl
Original file line number Diff line number Diff line change
Expand Up @@ -254,16 +254,19 @@ function splitpath(p::String)
return out
end

joinpath(path::AbstractString)::String = path

if Sys.iswindows()

function joinpath(path::AbstractString, paths::AbstractString...)::String
result_drive, result_path = splitdrive(path)
function joinpath(paths::Union{Tuple, AbstractVector})::String
assertstring(x) = x isa AbstractString || throw(ArgumentError("path component is not a string: $(repr(x))"))

isempty(paths) && throw(ArgumentError("collection of path components must be non-empty"))
assertstring(paths[1])
result_drive, result_path = splitdrive(paths[1])

local p_drive, p_path
for p in paths
p_drive, p_path = splitdrive(p)
p_path = ""
for i in firstindex(paths)+1:lastindex(paths)
assertstring(paths[i])
p_drive, p_path = splitdrive(paths[i])

if startswith(p_path, ('\\', '/'))
# second path is absolute
Expand Down Expand Up @@ -299,8 +302,15 @@ end

else

function joinpath(path::AbstractString, paths::AbstractString...)::String
for p in paths
function joinpath(paths::Union{Tuple, AbstractVector})::String
assertstring(x) = x isa AbstractString || throw(ArgumentError("path component is not a string: $(repr(x))"))

isempty(paths) && throw(ArgumentError("collection of path components must be non-empty"))
assertstring(paths[1])
path = paths[1]
for i in firstindex(paths)+1:lastindex(paths)
p = paths[i]
assertstring(p)
if isabspath(p)
path = p
elseif isempty(path) || path[end] == '/'
Expand All @@ -314,8 +324,12 @@ end

end # os-test

joinpath(paths::AbstractString...)::String = joinpath(paths)

"""
joinpath(parts::AbstractString...) -> String
joinpath(parts::Vector{AbstractString}) -> String
joinpath(parts::Tuple{AbstractString}) -> String
Join path components into a full path. If some argument is an absolute path or
(on Windows) has a drive specification that doesn't match the drive computed for
Expand All @@ -331,6 +345,11 @@ letter casing, hence `joinpath("C:\\A","c:b") = "C:\\A\\b"`.
julia> joinpath("/home/myuser", "example.jl")
"/home/myuser/example.jl"
```
```jldoctest
julia> joinpath(["/home/myuser", "example.jl"])
"/home/myuser/example.jl"
```
"""
joinpath

Expand Down
10 changes: 10 additions & 0 deletions test/path.jl
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@
@test joinpath(S("foo"), S(homedir())) == homedir()
@test joinpath(S(abspath("foo")), S(homedir())) == homedir()

for str in map(S, [sep, "a$(sep)b", "a$(sep)b$(sep)c", "a$(sep)b$(sep)c$(sep)d"])
@test str == joinpath(splitpath(str))
@test joinpath(splitpath(str)) == joinpath(splitpath(str)...)
end

if Sys.iswindows()
@test joinpath(S("foo"),S("bar:baz")) == "bar:baz"
@test joinpath(S("C:"),S("foo"),S("D:"),S("bar")) == "D:bar"
Expand All @@ -75,6 +80,11 @@
@test joinpath(S("\\\\server\\share"),S("a")) == "\\\\server\\share\\a"
@test joinpath(S("\\\\server\\share\\"), S("a")) == "\\\\server\\share\\a"

for str in map(S, ["c:\\", "c:\\a", "c:\\a\\b", "c:\\a\\b\\c", "c:\\a\\b\\c\\d"])
@test str == joinpath(splitpath(str))
@test joinpath(splitpath(str)) == joinpath(splitpath(str)...)
end

elseif Sys.isunix()
@test joinpath(S("foo"),S("bar:baz")) == "foo$(sep)bar:baz"
@test joinpath(S("C:"),S("foo"),S("D:"),S("bar")) == "C:$(sep)foo$(sep)D:$(sep)bar"
Expand Down

2 comments on commit fc02458

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.