- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 193
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
Execute additional command after -- #307
Conversation
fb3f095
to
6d73202
Compare
@@ -45,6 +46,29 @@ func printVersion(name string) { | |||
fmt.Printf("%s version %s\n", name, version.Version) | |||
} | |||
|
|||
// postRunExec - if templating succeeds, the command following a '--' will be executed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to not "overload" the --
to only mean execution. That way it can be used in other contexts down the line.
The way chamber
and aws-vault
work is they have an additional argument to indicate execution.
https://github.com/segmentio/chamber#exec
https://github.com/99designs/aws-vault#usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 do you have other contexts in mind?
I had a brief look at those other tools and it seems they both have an explicit exec
subcommand.
We could do that in gomplate
, but I'm not sure I really see the point right yet. At this point it seems like it'd just be an extra 4 characters to type 😉
There's nothing stopping alternate usage with a subcommand later on, while still supporting this more basic syntax.
Does that make sense @osterman?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - makes sense.
I look at it in terms of future proofing the command line interface. While today, it's adequate, it might reduce confusion in the long run. The way gomplate
is implemented is not unconventional or unreasonable. =) Just have noticed that amongst the apps I use these days, a common interface has started to evolve. If you look at helm
, kops
, kubectl
, chamber
, docker
, aws-valut
, terraform
, vault
, tctl
they all implement them this way. (They all happen to be in go
as well)
This is quickly spiraling into a larger conversation and just so you know, I don't feel incredibly strongly about it. I just wanted to point this out, since I think it's a nice interface to copy.
In gomplate
, the --version
option is available. There's no disputing that this is incredibly common of course in console apps. However, the trend I'm seeing to leverage a combination of positional parameter aka subcommand with options. I guess the thinking is options modify commands. So --version
doesn't modify anything. It's a command. This will make it easier as well to scope options to specific commands.
In the case of gomplate
, I could envision a number of subcommands.
e.g
validate
exec
version
apply
help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point @osterman, and, I'm on board. Maybe at some point later on I'll add some of those. I like the idea of validate
too, though not entirely certain that'd be easy to implement.
Anyway, for now we'll stick with this current naive/simplistic approach I think 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a bad idea, actually.
gomplate exec ${normal_gomplate_args} -- /my/command/and/its/args
Whereas the default subcommand is gomplate render
? Then it would be clear and easy to say, "the exec
subcommand requires the --
delimiter to separate gomplate
arguments from the command to be executed by gomplate
". If you don't specify exec
or render
, then you're getting the author's best guess at defaults and intentions.
Regardless, I'm excited for this.
6d73202
to
ed66ce6
Compare
Signed-off-by: Dave Henderson <[email protected]>
ed66ce6
to
2fefe92
Compare
Thanks @hairyhenderson ! |
Btw, will you be cutting a new release that includes this change? Would like to download the binary. =) |
@osterman I will - I think 2.5.0 is long overdue, so I'll be releasing that as soon as I get some time 😉 - hopefully tonight (EDT) |
We're using this here: https://github.com/cloudposse/default-backend/blob/master/Dockerfile#L11 Thanks! |
that's awesome, @osterman - I'm glad it's useful for you! 🎉 |
Fixes #300
Signed-off-by: Dave Henderson [email protected]