Merge #129

129: add `join` keyword to `readdir` r=mattBrzezinski a=ericphanson

This keyword was added to Base in JuliaLang/julia#33113, which made it into Julia 1.4.

I couldn't run the tests locally, but I tested with an example and it seemed to work. The new tests in `verify_files(path::AbstractPath)` need the Base method, so will fail on Julia 1 - 1.3. Should I put them behind a `VERSION` check, or just drop them?

Co-authored-by: ericphanson <[email protected]>

src/s3path.jl

@@ -322,17 +322,23 @@ function FilePathsBase.sync(f::Function, src::AbstractPath, dst::S3Path; delete=
     end
 end
 
-function Base.readdir(fp::S3Path)
+function Base.readdir(fp::S3Path; join=false, sort=true)
     if isdir(fp)
         k = fp.key
         # Only list the files and "dirs" within this S3 "dir"
         objects = s3_list_objects(fp.config, fp.bucket, k; delimiter="")
 
         # Only list the basename and not the full key
-        basenames = unique!([s3_get_name(k, string(o["Key"])) for o in objects])
+        results = unique!([s3_get_name(k, string(o["Key"])) for o in objects])
 
-        # Lexographically sort the results
-        return sort!(filter!(!isempty, basenames))
+        # Filter out any empty object names which are valid in S3
+        filter!(!isempty, results)
+
+        # Sort results if sort=true
+        sort && sort!(results)
+
+        # Return results, possibly joined with the root path if join=true
+        return join ? joinpath.(fp, results) : results
     else
         throw(ArgumentError("\"$fp\" is not a directory"))
     end

test/s3path.jl

@@ -272,18 +272,28 @@ end
 
     function verify_files(path::S3Path)
         @test readdir(path) == ["emptydir/", "subdir1/", "test_01.txt"]
+        @test readdir(path; join=true) == [path / "emptydir/", path / "subdir1/", path / "test_01.txt"]
         @test readdir(path / "emptydir/") == []
+        @test readdir(path / "emptydir/"; join=true) == []
         @test readdir(path / "subdir1/") == ["subdir2/", "test_02.txt", "test_03.txt"]
+        @test readdir(path / "subdir1/"; join=true) == [path / "subdir1/" / "subdir2/", path / "subdir1/" / "test_02.txt", path / "subdir1/" / "test_03.txt"]
         @test readdir(path / "subdir1/subdir2/") == ["subdir3/", "test_04.txt"]
+        @test readdir(path / "subdir1/subdir2/"; join=true) == [path / "subdir1/subdir2/" / "subdir3/", path / "subdir1/subdir2/" / "test_04.txt"]
         @test readdir(path / "subdir1/subdir2/subdir3/") == []
+        @test readdir(path / "subdir1/subdir2/subdir3/"; join=true) == []
     end
 
     function verify_files(path::AbstractPath)
         @test readdir(path) == ["emptydir", "subdir1", "test_01.txt"]
+        VERSION >= v"1.4.0" && @test readdir(path; join=true) == [path / "emptydir", path / "subdir1", path / "test_01.txt"]
         @test readdir(path / "emptydir/") == []
+        VERSION >= v"1.4.0" && @test readdir(path / "emptydir/"; join=true) == []
         @test readdir(path / "subdir1/") == ["subdir2", "test_02.txt", "test_03.txt"]
+        VERSION >= v"1.4.0" && @test readdir(path / "subdir1/"; join=true) == [path / "subdir1" / "subdir2", path / "subdir1" / "test_02.txt", path / "subdir1/" / "subdir1/test_03.txt"]
         @test readdir(path / "subdir1/subdir2/") == ["subdir3", "test_04.txt"]
+        VERSION >= v"1.4.0" && @test readdir(path / "subdir1/subdir2/"; join=true) == [path / "subdir1/subdir2/" / "subdir3", path / "subdir1/subdir2/" / "test_04.txt"]
         @test readdir(path / "subdir1/subdir2/subdir3/") == []
+        VERSION >= v"1.4.0" && @test readdir(path / "subdir1/subdir2/subdir3/"; join=true) == []
     end
 
     initialize()