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.

Sign up for GitHub

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

Jump to bottom

cli: switch from atty crate to crossterm #783

Merged
merged 1 commit into from
Nov 24, 2022
Merged

cli: switch from atty crate to crossterm #783

merged 1 commit into from
Nov 24, 2022

Conversation

martinvonz
Copy link
Member

The atty crate seems unmaintained. There's
https://rustsec.org/advisories/RUSTSEC-2021-0145 filed against it, which cargo-deny complains about. A fix for that has been open for well over a year without being fixed
(softprops/atty#51). The is-terminal crate is a fork of atty with that fixed, plus some other changes.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)

@martinvonz martinvonz enabled auto-merge (rebase) November 23, 2022 16:33
chooglen
chooglen reviewed Nov 23, 2022
View reviewed changes
src/ui.rs Outdated Show resolved Hide resolved
@martinvonz
Copy link
Member Author

This still fails the cargo-deny check because we also depend on atty via clap. There's clap-rs/clap#4249 for that. I'll see how to disable this check for now.\

@martinvonz martinvonz force-pushed the push-0b5afee10ab147d099ad3b4c2729b457 branch from 84aa1a4 to ba8e3f3 Compare November 23, 2022 17:40
@yuja
Copy link
Contributor

yuja commented Nov 24, 2022

We can also use crossterm for free.
https://docs.rs/crossterm/0.25.0/crossterm/tty/trait.IsTty.html

@martinvonz
Copy link
Member Author

We can also use crossterm for free. https://docs.rs/crossterm/0.25.0/crossterm/tty/trait.IsTty.html

Oh, that's even better. I'll make that change.

@martinvonz
cli: use is_tty() from crossterm crate instead of atty
bb3d480 
The `atty` crate seems unmaintained. There's
https://rustsec.org/advisories/RUSTSEC-2021-0145 filed against it,
which `cargo-deny` complains about. A fix for that has been open for
well over a year without being fixed
(softprops/atty#51). It turns out the
functionality is also available via the `crossterm` crate (thanks,
@yuja), which we already depend on.

Since we also depend on `atty` via `clap`, I also added an exception
to the `cargo-deny` config.
@martinvonz martinvonz force-pushed the push-0b5afee10ab147d099ad3b4c2729b457 branch from ba8e3f3 to bb3d480 Compare November 24, 2022 04:02
@martinvonz
Copy link
Member Author

We can also use crossterm for free. https://docs.rs/crossterm/0.25.0/crossterm/tty/trait.IsTty.html

Oh, that's even better. I'll make that change.

Done.

@martinvonz martinvonz merged commit 3e0f6ef into main Nov 24, 2022
@martinvonz martinvonz deleted the push-0b5afee10ab147d099ad3b4c2729b457 branch November 24, 2022 06:12
@martinvonz martinvonz changed the title cli: switch from atty crate to is-terminal cli: switch from atty crate to crossterm Nov 25, 2022
@martinvonz
Copy link
Member Author

Clap has now been migrated off of atty, but it turned out that we also depend on it via criterion, so we need to wait for a criterion release with bheisler/criterion.rs#628 before we can revert the change to deny.toml.

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

@chooglen chooglen chooglen left review comments

@yuja yuja yuja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinvonz @yuja @chooglen