Skip to content

std: clarify Command reuse docs#152143

Closed
npiesco wants to merge 1 commit intorust-lang:mainfrom
npiesco:main
Closed

std: clarify Command reuse docs#152143
npiesco wants to merge 1 commit intorust-lang:mainfrom
npiesco:main

Conversation

@npiesco
Copy link

@npiesco npiesco commented Feb 4, 2026

Args persist after execution; probe flags carry into later uses. Add note and example showing fresh Command pattern.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 4, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2026

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot

This comment has been minimized.

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 4, 2026
/// remaining unchanged; prefer creating a fresh builder once you know what you
/// need to run. Arguments and other settings remain on the builder after
/// execution, so probe-only flags will carry into later uses unless you build
/// a new `Command`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This documentation seems more muddling than clarifying: The case presented for making a new Command, probe-only flags, relies on the flags remaining unchanged. But it also says the internal state of Command is altered. Which is it?

@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 4, 2026
@workingjubilee
Copy link
Member

I saw your PR to servo.

r? @workingjubilee

@rustbot rustbot assigned workingjubilee and unassigned jhpratt Feb 4, 2026
Args persist after execution; probe flags carry into later uses. Add note and example showing fresh Command pattern.
@workingjubilee
Copy link
Member

workingjubilee commented Feb 4, 2026

While this is a library PR, I am responding to it in accord with MCP 893 because the principle, in my opinion, remains the same across the compiler and library: rust-lang/compiler-team#893

In servo/servo#42340 the PR you originally opened, you are being told by a Servo maintainer that there is no bug and that the change fixes nothing. You provide no justification. You cite no actual build failures. Your "clarification", especially including its example code, encourages a behavior that only makes sense if someone uses that very specific pattern. Otherwise, it is uselessly defensive programming that only makes sense if the implementation of Command is faulty, or if they fail to read the lines before and after this "clarification" that state for the "setting" functions, the Command is mutated, and that the spawned process will use the modified state.

As @sagudev observed: If there is an actual bug, we should fix it. However, in the absence of evidence of an actual bug, there is nothing to fix, and further review of this PR is unjustified, as this is a burdensome PR without motivation.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 4, 2026
@npiesco
Copy link
Author

npiesco commented Feb 5, 2026

While this is a library PR, I am responding to it in accord with MCP 893 because the principle, in my opinion, remains the same across the compiler and library: rust-lang/compiler-team#893

In servo/servo#42340 the PR you originally opened, you are being told by a Servo maintainer that there is no bug and that the change fixes nothing. You provide no justification. You cite no actual build failures. Your "clarification", especially including its example code, encourages a behavior that only makes sense if someone uses that very specific pattern. Otherwise, it is uselessly defensive programming that only makes sense if the implementation of Command is faulty, or if they fail to read the lines before and after this "clarification" that state for the "setting" functions, the Command is mutated, and that the spawned process will use the modified state.

As @sagudev observed: If there is an actual bug, we should fix it. However, in the absence of evidence of an actual bug, there is nothing to fix, and further review of this PR is unjustified, as this is a burdensome PR without motivation.

You're right. I reviewed the documentation better and I was wrong in trying to address it here. Closing and handling in servo build script where Windows non-TTY hang actually occurs. Thanks for feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants