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

replace ansi_term with yansi #102

Merged
merged 1 commit into from Aug 30, 2022

Conversation

Roguelazer
Copy link
Contributor

Fixes #101

Technically a breaking change because ansi_term::Style is publicly-exported (due to the way the macros are constructed), but I would be very surprised if anyone was relying on it.

@tommilligan
Copy link
Collaborator

Fixes #101

Thank you very much for taking this on and fixing it!

Technically a breaking change because ansi_term::Style is publicly-exported (due to the way the macros are constructed), but I would be very surprised if anyone was relying on it.

Agree that this is unlikely to break anything, I'm happy to include it in a minor patch version and cover it with a note in the changelog. I also think having a re-exported member is not required here, so I suggest we remove it altogether rather than have this problem again in the future.


I've pushed several commits to the branch, if you could take a look and give me your opinion on them:

  • Fix clippy lints
  • Remove the public export (as above)
  • You updated some of the printer internals to make style an Option<Style> - I don't think this is required and contains some subtle bugs. I've reverted it to Style

If you're happy with this review points, I'm happy to squash into a new release.

@tommilligan
Copy link
Collaborator

It also appears that yansi does not have a documented MSRV - I'll file an issue over there to ask what their support policy is likely to be.

@tommilligan
Copy link
Collaborator

Opened SergioBenitez/yansi#38, - yansi MSRV appears to be 1.54.0, which I'm happy to bump to as it's more than a year old at this point.

@Roguelazer
Copy link
Contributor Author

Your additions look fine to me

@tommilligan
Copy link
Collaborator

Your additions look fine to me

Thanks for taking a look. This is currently blocked on needing some more repo permissions - I have reached out to the owner to resolve this.

@extrawurst
Copy link

@colin-kiegel maybe giving others merge rights for the future?

@tommilligan tommilligan merged commit 3294a90 into rust-pretty-assertions:main Aug 30, 2022
@tommilligan
Copy link
Collaborator

Released in 1.3.0

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.

ansi_term unmaintained
3 participants