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

crossterm_utils fixes #211

Merged
merged 3 commits into from
Sep 14, 2019
Merged

crossterm_utils fixes #211

merged 3 commits into from
Sep 14, 2019

Conversation

zrzka
Copy link
Contributor

@zrzka zrzka commented Sep 13, 2019

  • Fixed doc examples
  • Optimised imports & warnings

cd crossterm_utils && cargo test is passing.

Signed-off-by: Robert Vojta <rvojta@me.com>
@zrzka zrzka changed the title crossterm-utils fixes crossterm_utils fixes Sep 13, 2019
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.

Nice work, I don't think we should mention crossterm_terminal, crossterm_cursor in the docstrings.

  1. when a user uses the crossterm crate, all types are reexported, and thus they belong to the crossterm namespace instead of crossterm_cursor/terminal.
  2. utils is not supposed to know anything about cursor and terminal. I can see a valid usecase for demonstration purposes, so we either use the commands from crossterm_utils like Output or we show types from cursor or terminal. But do we really need that dependency?

crossterm_utils/Cargo.toml Outdated Show resolved Hide resolved
crossterm_utils/src/macros.rs Outdated Show resolved Hide resolved
Signed-off-by: Robert Vojta <rvojta@me.com>
Signed-off-by: Robert Vojta <rvojta@me.com>
@zrzka zrzka mentioned this pull request Sep 13, 2019
@TimonPost TimonPost merged commit 6c320b1 into crossterm-rs:master Sep 14, 2019
@zrzka zrzka deleted the zrzka/utils-fixes 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.

2 participants