-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add trailing newline check for rustc --print
in run-make-support
#127887
Add trailing newline check for rustc --print
in run-make-support
#127887
Conversation
The run-make-support library was changed cc @jieyouxu This PR modifies cc @jieyouxu |
rustc --print
in run-make-support
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.
I don't really like the Vec<(String, Box<dyn Fn(&CompletedProcess)>)>
related extra complexity here. Why is the
assert!(out.stdout_utf8().ends_with('\n'));
check not sufficient?
This checks that every The #[derive(Debug)] // this works now
pub struct Command {
cmd: StdCommand,
stdin: Option<Box<[u8]>>,
assert_stdout_ends_with_newline: bool,
drop_bomb: DropBomb,
} and // Command::command_output
if self.assert_stdout_ends_with_newline {
completed_process.assert_stdout_ends_with_newline();
} |
@rustbot review |
pub struct Command { | ||
cmd: StdCommand, | ||
stdin: Option<Box<[u8]>>, | ||
completed_process_checks: Vec<(String, Box<dyn Fn(&CompletedProcess)>)>, |
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.
#[derive(Debug)] // this works now pub struct Command { cmd: StdCommand, stdin: Option<Box<[u8]>>, assert_stdout_ends_with_newline: bool, drop_bomb: DropBomb, }and
// Command::command_output if self.assert_stdout_ends_with_newline { completed_process.assert_stdout_ends_with_newline(); }
Let's use a simple bool for now, if there are more check-hooks that we wish to register upon construction, then we can always refactor into the hook form when it comes to it.
@rustbot author
☔ The latest upstream changes (presumably #128075) made this pull request unmergeable. Please resolve the merge conflicts. |
@Rejyr FYI: when a PR is ready for review, send a message containing Or if you're not going to continue, please close it. Thank you! |
I think checking |
From this comment.
Checks that
rustc --print
stdout ends with a newline.Adds
CompletedProcess
check closures toCommand
.Unfortunately, this means that
#[derive(Debug)]
macro forCommand
no longer works.Part of #121876.
r? @jieyouxu