Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support interpolating in git cmd string macro #30

Closed
fredrikekre opened this issue Dec 13, 2019 · 15 comments · Fixed by #36
Closed

Support interpolating in git cmd string macro #30

fredrikekre opened this issue Dec 13, 2019 · 15 comments · Fixed by #36

Comments

@fredrikekre
Copy link

No description provided.

@DilumAluthge
Copy link
Member

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
Copy link
Member

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
Copy link
Member

JuliaLang/julia#23481

@DilumAluthge
Copy link
Member

The discussion in JuliaLang/julia#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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link

omus commented Sep 9, 2020

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

@DilumAluthge DilumAluthge transferred this issue from JuliaVersionControl/GitCommand.jl Mar 23, 2021
@GunnarFarneback
Copy link
Contributor

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
Copy link
Author

Isn't it better to just let people use

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

?

@GunnarFarneback
Copy link
Contributor

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
Copy link
Author

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

@GunnarFarneback
Copy link
Contributor

GunnarFarneback commented Mar 24, 2021

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
Copy link
Member

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
Copy link
Member

Step 1 is done in #34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants