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

stdin handling is an unfriendly API #73

Closed
epage opened this issue Mar 21, 2019 · 1 comment · Fixed by #85
Closed

stdin handling is an unfriendly API #73

epage opened this issue Mar 21, 2019 · 1 comment · Fixed by #85
Labels
breaking-change bug Not as expected

Comments

@epage
Copy link
Contributor

epage commented Mar 21, 2019

Based on #71

The automatic reaction is to write something like

let mut cmd = Command::new("cat");
cmd
    .with_stdin()
    .buffer("42");
let assert = cmd.assert();
assert
  .success()
  .stdout("42");

When you really need to write

 let mut cmd = Command::new("cat");
 cmd
     .arg("-et");
 cmd
     .with_stdin()
     .buffer("42")
     .assert()
     .stdout("42");

The problem is an issue with &mut builders (we can't chain off the original cmd), assuming with_stdin will act like &mut builder function when in reality you should only be acting on it at that point, etc.

Note: The lib.rs documentation still has the broken approach

@epage epage added bug Not as expected breaking-change labels Mar 21, 2019
@epage
Copy link
Contributor Author

epage commented Mar 21, 2019

Ideas

  • Switch with_stdin to take ownership to ensure it is use correctly
  • Add must_use to things so people get compiler errors

Either way, the need to split this into three statements is not resolved.

epage pushed a commit to epage/assert_cmd that referenced this issue Dec 5, 2019
Many of our problems with `stdin` are derived from using extension
traits. By moving away from them, we fix the API problems and make it
easier to add other features like timeout (assert-rs#10) or signalling (assert-rs#84).

So the new philosphy if:
- Core functionality is provided by extension traits
- Provide a convinience API that makes `Command` friendlier. These do
  not need to be generalized.  Other abstractions can provide their own
  (like `duct`).

Fixes assert-rs#73
@epage epage closed this as completed in #85 Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change bug Not as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant