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

proposal: os/exec: add method Cmd.String #30638

Closed
josharian opened this issue Mar 6, 2019 · 8 comments
Closed

proposal: os/exec: add method Cmd.String #30638

josharian opened this issue Mar 6, 2019 · 8 comments
Labels
FrozenDueToAge help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Milestone

Comments

@josharian
Copy link
Contributor

It is common to want to log a command that you are about to run (or a command that you just ran, on error). Currently to do that you have to combine Cmd.Path and strings.Join Cmd.Args. I propose that we add a String method to Cmd that implements this very common need.

@acln0
Copy link
Contributor

acln0 commented Mar 6, 2019

I've implemented this a couple of times myself, so I'd like to see it in os/exec too.

@josharian
Copy link
Contributor Author

As a bonus we could shell-quote args as needed for clarity. But too far down that path lies madness. The intent is human readable output only. We’d document that.

@josharian josharian added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 6, 2019
@odeke-em odeke-em changed the title os/exec: add method Cmd.String proposal: os/exec: add method Cmd.String Mar 6, 2019
@gopherbot gopherbot added this to the Proposal milestone Mar 6, 2019
@myitcv
Copy link
Member

myitcv commented Mar 7, 2019

I've often wanted this as well.

But I've also often extended such a string to be prefixed by the environment variables I added atop os.Environ(), so the resulting string becomes something like:

GOBIN=/some/path GOPROXY= go install example.com/blah

This is clearly not an argument against String() as proposed, but more a question on how often it would end up being useful.

@acln0
Copy link
Contributor

acln0 commented Mar 7, 2019

@myitcv I have done that in the past, in places where I knew cmd.Env was a superset of os.Environ(). But I don't know how it could be generalized, since cmd.Env can contain arbitrary values, and if cmd.Env is not a superset of os.Environ, then printing them all might be a little too chatty. If exec.Cmd implemented fmt.Formatter, perhaps the extra environment variables could be added to the string if %+v is specified, or something. But this whole thing feels a little beyond the scope of the current issue.

@myitcv
Copy link
Member

myitcv commented Mar 7, 2019

But this whole thing feels a little beyond the scope of the current issue.

Totally. Just to confirm, I was not advocating broadening the scope, just raising the question.

@josharian
Copy link
Contributor Author

ISTM that this is still useful: you’d print the appropriate env vars and then use String to print the rest. But perhaps I misunderstand the question.

@ianlancetaylor
Copy link
Contributor

Accepted per @golang/proposal-review .

@ianlancetaylor ianlancetaylor added Proposal-Accepted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 20, 2019
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Unplanned Mar 20, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 20, 2019
@ianlancetaylor ianlancetaylor added help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 20, 2019
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Mar 20, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/168518 mentions this issue: os/exec: add Cmd.String

@golang golang locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

5 participants