adds_function_relpath, docs, tests

[peter1000]

hi @tkelman

This adds function relpath which has similar functionalities as python's os.path.relpath

Return a relative filepath to path either from the current directory or from an optional start directory. This is a path computation: the filesystem is not accessed to confirm the existence or nature of path or start.

I tested it first with expected results generated by python's implementation to be on the save side (hopefully).

Cheers
P

At the moment this is the last julia File related function I thought would be worthy to be included.

e.g. Docile/Lexicon uses it already Lexicon.jl

[tkelman]

Interesting. I don't see why you pinged me specifically, but this does look like a useful function to have and a nicely thorough PR.

[peter1000]

I had only contact with you and you seem the one who does follow this things through?

[tkelman]

Sure. If this passes tests on CI and nobody has any comments for a few days I think this could be merged as-is, thanks for the contribution. Is this exactly the same code as @MichaelHatherly wrote in Lexicon.jl?

[peter1000]

It is the same code I contributed to Lexicon.jl. I will update the PR as I forgot to export relpath.

[tkelman]

One minor nitpick, it's a little misleading to have this under the comment heading # The following use Unix command line facilites, maybe move it to the end of the file instead?

[peter1000]

ok - do you want an entry in NEWS.md under Library: File

probably the `readlink, and cp addition( julia 3.7 has no keyward arguments) should also be there ? But I can also update the news later if you prefer

[tkelman]

Sure, I guess there have been enough additions under the category of filesystem stuff that it deserves its own news section, it didn't fit in too well with the existing ones last time I skimmed through them.

Looks like Travis isn't happy that it isn't exported. Adding a new export should usually be done somewhat cautiously. But I think for now it does fit to have this kind of operation exported, and later down the line we can consider moving all filesystem-related functions into the FS module. Would appreciate a second opinion on this.

[dhoegh]

👍 For adding a FS module, with file/path related functions, that would reduce the namespace clutter and make the functions easier to find.

[tkelman]

it's a little misleading to have this under the comment heading # The following use Unix command line facilites, maybe move it to the end of the file instead?

On second thought since this isn't really a filesystem operation at all, wouldn't it fit better in base/path.jl rather than base/file.jl?

[peter1000]

you are right - I will move that to path.jl. and update the PR

[peter1000]

Updated the PR.

• Moved as suggested the new function relpath to path.jl.
• Improved the documentation by adding the return type.
• Changed the Arguments from ByteString to AbstractString to be in line with the other functions.
• Removed not needed Base. for some items.
• Added to the checks also Unicode path.
• Added NEWS.md entry for relpath under Library > Other improvements

[tkelman]

the 32 bit appveyor failure is unrelated, but on 64 bit appveyor https://ci.appveyor.com/project/StefanKarpinski/julia/build/1.0.3938/job/vibrnxrimbxe7bve it looks like you need to use the platform path separator in your expected results

[tkelman]

Alright I think we can merge this now. Anyone have an opinion here? @StefanKarpinski maybe? Is this useful enough to warrant having it in base, and exported for now?

[StefanKarpinski]

Seems reasonable to me. Most of the rest of the path and file API are modeled on Python, so this seems fitting. The thorough testing makes me feel pretty comfortable merging as well – thanks, @peter1000!

[tkelman]

Apparently this needs to be rebased again though.

[peter1000]

I will do that in a couple of minutes. Updated the PR: recompiled and run all tests previously.