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

Doc bug in Command::stdin? #6577

Closed
roy-work opened this issue May 21, 2024 · 1 comment · Fixed by #6581
Closed

Doc bug in Command::stdin? #6577

roy-work opened this issue May 21, 2024 · 1 comment · Fixed by #6581
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-process Module: tokio/process T-docs Topic: documentation

Comments

@roy-work
Copy link
Contributor

Version
1.37.0

Platform
N/A / the documentation

Description

The documentation for Command::stdin reads,

Sets configuration for the child process’s standard input (stdin) handle.

Defaults to inherit when used with spawn or status, and defaults to piped when used with output.

N.b., that this documentation is essentially copy/pasted between stdin, stdout, and stderr, with references to stdin above replaced appropriately; I think this is a bug in that stdin's docs were merely copy/pasted from stdout/stderr without enough thought; in particular, let's look at the second paragraph.

In the second paragraph: assume we call output, which captures the output of the process: why should such a call presume piped as a default for stdin? (It would make sense for stderr and stdout. But stdin?)

If we look at the source for output:

    pub fn output(&mut self) -> impl Future<Output = io::Result<Output>> {
        self.std.stdout(Stdio::piped());
        self.std.stderr(Stdio::piped());

        let child = self.spawn();

        async { child?.wait_with_output().await }
    }
}

… this seems to suggest that stdin is not piped merely by calling Command::output.

I think the behavior is fine, but the docs seem to not relay the correct behavior. This paragraph should probably just be truncated to "Defaults to inherit." for stdin only.

@roy-work roy-work added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels May 21, 2024
@mox692
Copy link
Member

mox692 commented May 22, 2024

This paragraph should probably just be truncated to "Defaults to inherit." for stdin only.

Such a document update sounds good to me.

@mox692 mox692 added T-docs Topic: documentation M-process Module: tokio/process labels May 22, 2024
mox692 pushed a commit that referenced this issue May 23, 2024
The code for `output` indicates that only sets `stdout` and `stderr`, 
yet the docs for `stdin` indicated that it too would be set. This seems 
like it was just a simple copy/paste typo, so correct `stdin` to note
that it just defaults to `inherit`.

Fixes #6577.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-process Module: tokio/process T-docs Topic: documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants