Add @__DIR_P__, @__FILE_P__ and @LOCAL macro

[davidanthoff]

CC @vtjnash

There are some open questions before we can merge this:

• I'm not happy with the names @__DIR_P__ and @__FILE_P__, does anyone have a better idea?
• What do we want to return if this is run from the REPL or julia -e? Right now it always returns Path(), but I'm not sure that is a good idea. Should we instead return nothing?

[rofinn]

I'd opt for just leaving them unexported for now and keeping the same names as base. Alternatively, I'd go with @__PATH__ and @__FILEPATH__.

[vtjnash]

I'm not sure why you're pinging me, since I don't currently use this package or currently have any needs that are addressed by it.

[davidanthoff]

@vtjnash I'm pinging you because you added @LOCAL to Electron.jl, where I will remove it soon once this PR here is merged. No need to review anything, I just wanted to let you know where you can find a macro similar to what you had added going forward.

@rofinn I'll go with your new name recommendations, I like them! I do think exporting by default makes sense, after all the whole point of this is to have some short things.

[codecov-io]

Codecov Report

Merging #1 into master will increase coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master       #1      +/-   ##
==========================================
+ Coverage   82.51%   82.64%   +0.12%     
==========================================
  Files           8        8              
  Lines         406      409       +3     
==========================================
+ Hits          335      338       +3     
  Misses         71       71

Impacted Files	Coverage Δ
src/FilePathsBase.jl	28.57% <ø> (ø)	⬆️
src/path.jl	87.27% <100%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3dee7b5...7fd4a07. Read the comment docs.

[rofinn]

This seems fine apart from a 0.7 syntax issue. Now that I think about it, having just @__PATH__/@__CWD__ and @__FILEPATH__ might be best, as @__FILEPATH__ could just default to the current file if one isn't specified?

[rofinn]

You'll need spaces in the ternary for that to be valid syntax on 0.7.

[davidanthoff]

Done

[davidanthoff]

Now that I think about it having just @__PATH__/@__CWD__ and @__FILEPATH__ might be best as @__FILEPATH__ could just default to the current file if one isn't specified?

Alright, I have stared at this for a minute, and I just don't understand what this sentence means :) Can you elaborate a bit what you are suggesting?

[rofinn]

Sorry, I was just suggesting that we have filespec be an optional argument to the @__FILEPATH__ macro, rather than having a separate @LOCAL macro just for that behaviour.

[davidanthoff]

Ah :) Hm, I almost feel having three separate things is cleaner? I'm not too keen on the name @LOCAL, but I think keeping @__FILEPATH__ and @__PATH__ really simple and in line with what is in base would be easier for folks to grok? I think if @__FILEPATH__ did both things it would feel a bit too magical for my tastes?

I do wonder whether there is a better name for @LOCAL, though...

Or, here is another thought: if we used some operator for path concatenating (say *), maybe we could just do without @LOCAL altogether? Say we had:

function Base.:*(a::AbstractPath, b::AbstractPath)
    return join(a,b)
end

function Base.:*(a::AbstractPath, b::AbstractString)
    return join(a,Path(b))
end

Then one could just write

@__PATH__() * "foo"

Which looks quite natural to me?

[rofinn]

I could see introducing / (or maybe ‖) as a join operator.

@__PATH__() / "foo"

or

@__PATH__() ‖ "foo"

The benefit is that it wouldn't be confused with string concatenation and both seem more correct for than * (especially for paths).

[davidanthoff]

How about I remove @LOCAL for now, so that we can merge the simple things now and iterate a bit more on the other stuff?

[rofinn]

Sure, would you be okay with not exporting @LOCAL for now? That way we don't need go through a proper deprecation cycle.

[davidanthoff]

Alright, now @LOCAL is now no longer exported. I think this could be merged once CI passes, right?