-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
tty: expose hasColor
and getColorDepth
directly
#40240
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
base: main
Are you sure you want to change the base?
Conversation
hasColor
and getColorDepth
directlyhasColor
and getColorDepth
directly
8695301
to
69b6d1d
Compare
I feel that a good amount of people agree with the idea of deprecating |
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.
@thunder-coding, thanks for the PR. I definitely see value in exposing this to help terminal color library authors write smaller modules and would like to see this happen, but this PR still seems to be missing quite a few things.
Maybe if you added some documentation to this PR (and your commit message adhered to our commit message guidelines this might be able to get some traction, but until then, this is unlikely to get approval.
With regards to deprecating things, the option of documentation-deprecating something so as to not break pre-existing uses is an option that would be available if necessary. Let me know if you get stuck or need help with anything if you intend to continue pursuing this.
69b6d1d
to
57223a8
Compare
I have rebased this against master and even fixed the commit message. About documentation-deprecating Note: I only need help regarding how to write the deprecation thing in |
Hey, I'm interested in seeing progress on this PR. Any way to help? |
Thanks for showing interest in my PR. I would like to continue and apply the requested changes and get this merged. But I've got quite busy nowadays due to my exams, and I won't be able to get back to this for atleast the next 2 months! So, if anyone would like to take over the opportunity to create a new PR which implements it properly is free to do so! |
TODOs:-
hasColor
getColorDepth
- [x] DeprecateWriteStream.hasColor
andWriteStream.getColorDepth
- [x] Add tests for same- [ ] Replace every instance ofWriteStream.hasColor
andWriteStream.getColorPath
with new methods intty
- [ ] Document new deprecated methods oftty
and add documentation for new methodsAbout deprecating
tty.WriteStream.hasColor
andtty.WriteStream.getColorDepth
, this requires a lot of changes to be done which is not so backward compactible.process.stdout
andprocess.stderr
havehasColor()
andgetColorDepth()
methods which are forwarded fromtty.WriteStream
, this means that deprecatingtty.WriteStream.hasColor()
will also mean deprecation ofprocess.stdout.hasColor
, similar forprocess.stdout
andgetColorDepth)
So I am dropping the idea of deprecating these two methods for now. I would really appreciate some feedback from the team regarding this.
Fixes #40236