JuliaVersionControl / Git.jl

Use command-line Git in your Julia packages
MIT License
36 stars 8 forks source link

Support interpolating in git cmd string macro #30

Closed fredrikekre closed 3 years ago

DilumAluthge commented 4 years ago

I think this is a bug in Julia?

julia> macro foo_cmd(ex)
       println(ex)
       end
@foo_cmd (macro with 1 method)

julia> a = "123"
"123"

julia> foo`x y z`
x y z

julia> foo`x $a z`
x $a z
DilumAluthge commented 4 years ago
julia> macro foo_str(ex)
       println(ex)
       end
@foo_str (macro with 1 method)

julia> macro bar_cmd(ex)
       println(ex)
       end
@bar_cmd (macro with 1 method)

julia> a = "hello"
"hello"

julia> foo"$a"
$a

julia> bar`$a`
$a
DilumAluthge commented 4 years ago

https://github.com/JuliaLang/julia/issues/23481

DilumAluthge commented 4 years ago

The discussion in https://github.com/JuliaLang/julia/issues/23481 seems to imply that support for interpolation string macros won’t be added to Julia.

So we’d have to add the support ourselves in this package.

fredrikekre commented 4 years ago

So we’d have to add the support ourselves in this package.

Yea, thats what I meant from the start, should have been more clear.

DilumAluthge commented 4 years ago

So we’d have to add the support ourselves in this package.

Yea, thats what I meant from the start, should have been more clear.

Nah, the confusion was all on me.

Anyway, sounds like a good idea. I don’t really know enough about macros to figure out how to don’t this; maybe someone else can help out.

fredrikekre commented 4 years ago

I haven't looked at it, but can maybe look at what @btime etc does since they also use $ in macros to men interpolation (although for a different purpose).

omus commented 4 years ago

I had to deal with handling interpolation in a non-standard string literal here: https://github.com/invenia/MultilineStrings.jl/blob/3192dbf8872b99cb2a09a46894b22428191805ca/src/MultilineStrings.jl#L120. I think the same approach would probably work here

GunnarFarneback commented 3 years ago

I propose to deprecate the cmd string macro in favor of git("clone https://github.com/JuliaRegistries/General") and split the argument with Base.shell_split. This gives interpolation for free and allows the use of whatever string macros people want to use. (I already have a PR in preparation for git(::AbstractString)).

fredrikekre commented 3 years ago

Isn't it better to just let people use

git = Git.git()
`$git clone ....`

?

GunnarFarneback commented 3 years ago

That's already available so if we think that's all that's needed the easiest solution is just to remove the cmd macro.

fredrikekre commented 3 years ago

Exactly. Is there any other advantage with the cmd macro?

GunnarFarneback commented 3 years ago

Considering that it currently does a plain split of the argument I would say that it's mostly kinda dangerous but for one time uses it's a bit more compact than first having to create the git object or interpolating it into the command. ~My proposal would allow things like git(raw"status src\foo\bar.jl") which might be practical if you e.g. need to copy/paste paths on Windows.~ Never mind, shell_split destroys the backslashes.

The functionality I really want, however, is git(["status", "README.md"]) to bypass the cmd argument parsing. But maybe the magic of the cmd interpolation of iterables doesn't make it worth the trouble.

DilumAluthge commented 3 years ago

It sounds like the plan here would be:

  1. Remove the @git_cmd cmd macro. We can do this in a nonbreaking release because it's not part of the public API.
  2. Add a new method git(::AbstractVector(<:AbstractString)) for the use case that @GunnarFarneback wants.
DilumAluthge commented 3 years ago

Step 1 is done in #34