Add integration with URIParser #22
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22 +/- ##
=========================================
Coverage ? 72.72%
=========================================
Files ? 1
Lines ? 11
Branches ? 0
=========================================
Hits ? 8
Misses ? 3
Partials ? 0
Continue to review full report at Codecov.
|
|
How would you feel about having a FilePathsExt.jl package that could use Requires.jl to handle optional dependencies on packages like this (e.g., Glob.jl, URIParser.jl)? |
|
Yes, that would certainly work. Another option might be to rename this package here into FilePathsBase.jl, and then have FilePaths.jl include that stuff. I don't really care, but it would mean that things would just work if folks use FilePaths.jl. Happy to go with either option. If we go with your idea, do you want to create the repo, or should I do it? I almost think we could actually skip Requires.jl for that one and just make FilePaths, URIParser and Glob a dependency? Neither of those is heavy weight, and it might just be easier. |
|
Okay, I'll create that package tomorrow. I'm fine with renaming this package to FilePathsBasse.jl, but I'm not sure how that renaming would work on METADATA? Would the following work?
|
|
We should probably ask someone who knows how that works, but here is one way that probably works: You create a new FilePathsBase.jl repo. You then just push the master branch from your local clone of FilePaths.jl to that new FilePathsBase.jl repo. Now both repos have the same commit as their master. Then you rename things in FilePathsBase.jl repo and register that package. In FilePaths.jl you add a commit that deletes everything except the integrations stuff. The only thing in METADATA then would be registering FilePathsBase.jl as a new package, and then some new tags. I think that might be the least risky path? |
|
Okay, once FilePathsBase.jl is registered I'll update FilePaths.jl to just reexport those symbols and we can add URIParser and Glob back in. Would you be interested in supporting FileIO.jl? |
|
Hurray, that is excellent! Yes, it would be nice if we could integrate with FileIO. At the same time I almost feel FileIO is ripe for a bit of a redesign. It is not entirely clear to me whether all the fancy "load packages behind the scene" stuff will still work on 0.7. But I might be wrong. I think the first thing I'll try is to use this in the VS Code extension where constantly have to juggle with file paths and URIs and it has to work on all platforms. |
src/uri.jl
Outdated
|
|
||
| for i=2:length(p.parts) | ||
| print(b, "/") | ||
| print(b, p.parts[i]) |
|
I think this can also be merged now. |
src/uri.jl
Outdated
| @@ -0,0 +1,20 @@ | |||
| function URIParser.URI(p::AbstractPath) | |||
| if isempty(root(p)) | |||
| error("$p is not an absolute path") | |||
There was a problem hiding this comment.
Probably want to throw an ArgumentError here and we should test that error condition.
| b = IOBuffer() | ||
| print(b, "file://") | ||
|
|
||
| if !isempty(drive(p)) |
There was a problem hiding this comment.
We should probably test the drive behaviour.
|
Alright, I addressed both points you raised. |
|
Thanks, merging. I'm not sure what the deal with codecov is, but I've updated the README and if this happens on another PR then I'll dig into that more. |
|
Could you also tag a release? |
Fixes #18.
@rofinn, I completely agree with your general sentiment in #18, but am wondering whether we can merge this nevertheless for now.
Here the logic: I don't think it is realistic that this package might be made part of the stdlib for julia 1.0, that strikes me like a 1.1 project. I also feel that FilePaths is not stable enough at this point to ask URIParser to take a dependency on it. So I think as an intermediate step it makes most sense to have this functionality here in FilePaths.
Once FilePaths is ready to be moved into the stdlib (like, in a year or so), we can move the integration out of it and into URIParser.
Does that sound reasonable to you?