-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Use nu-ansi-term
instead of ansi_term
#2511
Conversation
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.
ansi_term
is considered abandoned, so this is a good change, so thank you.
Had some comments.
Cargo.toml
Outdated
@@ -42,7 +42,7 @@ regex-fancy = ["syntect/regex-fancy"] # Use the rust-only "fancy-regex" engine | |||
|
|||
[dependencies] | |||
atty = { version = "0.2.14", optional = true } | |||
ansi_term = "^0.12.1" | |||
ansi_term = { version = "0.47", package = "nu-ansi-term" } |
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.
Please specify version as 0.47.0 please (see https://users.rust-lang.org/t/psa-please-specify-precise-dependency-versions-in-cargo-toml/71277)
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.
done
src/bin/bat/main.rs
Outdated
@@ -14,7 +14,7 @@ use std::io::{BufReader, Write}; | |||
use std::path::Path; | |||
use std::process; | |||
|
|||
use ansi_term::Colour::Green; | |||
use ansi_term::Color::Green; |
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.
Cool trick that you can keep the old crate name. I didn't know about it.
But long-term I think it will be confusing, so I would prefer if you didn't keep ansi_term
around.
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.
yeah, I went for the minimal changes.
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.
done and all green.
The `nu-ansi-term` crate is a fork of `ansi_term` which is maintained by the Nushell project.
@nickelc All of these maintenance PRs of yours are very much appreciated! I really like that they come in small, well-structured individual changesets! This change looks good to me. Do we "leak" any of these @Enselic has a nice tool to check for this: https://github.com/Enselic/cargo-public-api |
Here is the output of Details
|
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.
Thank you
The
nu-ansi-term
crate is a fork ofansi_term
which is maintained by the Nushell project.Highlights:
winapi
dependency withwindows-sys
.See
CHANGELOG.md
for the changes since forking it.