Skip to content

proposal: os/exec: add String() to *exec.Cmd #21707

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

Closed
hirochachacha opened this issue Aug 31, 2017 · 9 comments
Closed

proposal: os/exec: add String() to *exec.Cmd #21707

hirochachacha opened this issue Aug 31, 2017 · 9 comments

Comments

@hirochachacha
Copy link
Contributor

I propose adding String() to *exec.Cmd. For example,

cmd := exec.Command("ls", "-l")
fmt.Printltn(cmd)

will print

ls -l

or

/bin/ls -l

This is useful when we want to improve error messages.

@gopherbot gopherbot added this to the Proposal milestone Aug 31, 2017
@robpike
Copy link
Contributor

robpike commented Aug 31, 2017

It's pretty easy to do this yourself if you're in charge of constructing the command line.

@odeke-em
Copy link
Member

Yeah, same thought as @robpike, and in deed you have better control yourself, and can
directly inspect cmd.Args which would even be more helpful for things like structured logging e.g.

log.Error("exec", "args", cmd.Args)

@hirochachacha
Copy link
Contributor Author

In general, we can write any program without String().
We can always write own string representation easily. String() is not an intrinsic method.
But it doesn't mean String() is useless, programmers are sometimes lazy.
I feel that *exec.Cmd is a good candidate for fmt.Stringer because the string representation can express differences between each instances effectively.

@davecheney
Copy link
Contributor

davecheney commented Aug 31, 2017 via email

@robpike
Copy link
Contributor

robpike commented Aug 31, 2017

I disagree. A Cmd is a lot more (a LOT!) than a set of arguments, and implementing Stringer for that type would, by this approach printing only the args, suppress the rest.

@hirochachacha
Copy link
Contributor Author

Should Sting() keep all information? I thought that is more like GoString()'s job.

I can see text/template/parse/node.go. Those AST have own String() method.
Even though those are loosing position information, I suppose those are still useful and their String() express themselves enough.

Similarly, I think cmd+args express exec.Command enough.
We only provide func Command(name string, arg ...string) *Cmd and its context version as constructors because those arguments are intrinsic, and other attributes are optional.

@hirochachacha
Copy link
Contributor Author

(Maybe "text/template/parse" is not a good example. I cannot imagine good examples. Sorry.)

@rsc
Copy link
Contributor

rsc commented Oct 16, 2017

Rob's comment #21707 (comment) seems like the most important argument against - why should an exec.Cmd stringify as the command being run? Why not the result? Why not some other detail of the struct?

You could argue that it should have a ShellCommand() string method or something like that, to get just a printable command, but even that is very difficult and not clearly a good idea, since exec.Cmd doesn't use the shell to execute the command. (Well, except on Windows.)

Declining proposal.

@hirochachacha
Copy link
Contributor Author

I see. Thank you for your opinions.

@golang golang locked and limited conversation to collaborators Oct 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants