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

write_cout! fix #212

Merged
merged 8 commits into from
Sep 14, 2019
Merged

write_cout! fix #212

merged 8 commits into from
Sep 14, 2019

Conversation

zrzka
Copy link
Contributor

@zrzka zrzka commented Sep 13, 2019

DON'T MERGE as it needs to be rebased after all other PRs will be merged (if they will be) and there will be more occurrences of unused std::io::Write imports.

  • write_cout! returns number of written bytes
  • Terminal::write returns number of written bytes (result of write_cout!)
  • execute! macro is a bit simpler
  • Remove unused std::io::Write imports

NEW

  • Fixes rest of the _cursor & _style to make the cargo test --all --exclude crossterm_winapi pass (didn't check _winapi yet)

Fixes #202

@zrzka
Copy link
Contributor Author

zrzka commented Sep 13, 2019

Or there's 2nd variant of the write_cout! which does use write_all instead of write.

The 1st variant with write can fail, can write part of the buffer only, can return interrupted, which is not an error basically, etc.

Copy link
Member

@TimonPost TimonPost left a comment

Choose a reason for hiding this comment

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

Thanks, Looks good to me! Still a WIP?

error = Some(Err($crate::ErrorKind::from(e)));
}
_ => {}
if let Err(e) = $command.execute_winapi() {
Copy link
Member

Choose a reason for hiding this comment

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

Yea, definately better :)

@zrzka
Copy link
Contributor Author

zrzka commented Sep 13, 2019

Yeah, still WIP. Waiting for other PRs to rebase & I didn't go through examples, etc. to check if there is/isn't an unused import.

@zrzka
Copy link
Contributor Author

zrzka commented Sep 13, 2019

Will fix these conflicts with rebase when the #211 & #207 will be merged.

Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
@zrzka zrzka changed the title [WIP] write_cout! fix write_cout! fix Sep 14, 2019
@zrzka
Copy link
Contributor Author

zrzka commented Sep 14, 2019

@TimonPost rebased, added more fixes (see first comment) and finally the whole cargo test --all --exclude crossterm_winapi is passing on my computer. Will continue with 2018 + examples collisions + other cleanup in another branch.

@TimonPost TimonPost merged commit be05974 into crossterm-rs:master Sep 14, 2019
@zrzka zrzka deleted the zrzka/write-cout-fix branch September 16, 2019 05:47
december1981 pushed a commit to december1981/crossterm that referenced this pull request Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terminal::write -> Result<usize>
2 participants