-
-
Notifications
You must be signed in to change notification settings - Fork 256
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
deinit() doesn't work if init() was called twice. #145
Comments
I oringinally thought that this problem could be solved by just running However this does not seem to be the case when I started investigating, I found out that the second >>> import colorama
>>> import sys
>>> id(sys.stdout) # The original stdout output
56145200
>>> colorama.init()
>>> id(sys.stdout) # The stdout output after init()
94884112
>>> colorama.deinit()
>>> id(sys.stdout) # Should be back to the original stdout output
56145200 # stdout goes back
>>> colorama.init()
>>> id(sys.stdout) # The stdout output after init()
95230320
>>> colorama.init() # init() for the second time
>>> id(sys.stdout) # The stdout output after the second init()
95230544
>>> colorama.deinit() # deinit() once
>>> id(sys.stdout) # stdout output after first deinit()
95230320 # stdout goes back to stdout after first deinit()
>>> colorama.deinit() # deinit() again (second time), should go back to original stdout ouput
>>> id(sys.stdout) # stdout output after second deinit()
95230320 # stdout output is the same after the second deinit() as after the first deinit() For the curious, the same effect occurs when more calls to I hope this helps, I think an error needs to be raised when init() is called again before deinit() is called to prevent this problem happening. |
If memory serves, |
I'm worried about calling Should |
the bug is caused by tartley/colorama#145
To @gilch, Sep 26, 2017
|
@AlkisPis Play nice, m'kay? Glitch and everyone else are doing their best, giving time and effort for free, we should all be grateful, not demanding. I'll see if I can figure this out in the next few days. Hugs, all |
I'm thinking of fixing this by making 'init()' a no-op if it has already wrapped stdout. This has some precedence, because 'init()' is already a no-op in other situations, such as on non-windows platforms. So we'll only need a single 'deinit()' to fix multiple 'init()' calls. |
Also, @AlkisPis, the above means that text colors and style will not be reset to regular on calling 'deinit()'. Calling 'deinit' never does this, no matter whether 'init' was called twice or just once. If an application wants them reset, it should print an ANSI 'reset' code, e.g. by printing Colorama's Style.RESET_ALL, before calling 'deinit()'. |
I used colorama to pretty log and was very cool to have multi-platform support. Hope to use it more times in the future. I liked the idea of init became no-op, the user interface would be very clean, @tartley. Another option would be to wrap |
Yuck, this code I wrote is rubbish. Sorry people. :-) Upvotes on this comment are acceptable. |
There is a complication. Current behavior is that calling 'init' more than once, besides breaking 'deinit', can also override the passed values of kwargs. (autoreset, convert, strip, wrap) So simply making the 2nd 'init' a no-op would break this existing behavior. I'll see if I can fix the issue while preserving this behavior... |
Can someone with a Windows machine try out the branch above? Run tests and review the changes? Many thanks. |
For me seems alright but no Windows around here either. |
Why am I not playing nice? Anyway, this is an opportunity for me to thank you for your nice package! :) |
@tartley, I have a Windows OS. What "branch" exactly to test? I visited #236 but it's a confusion (for me). Can you just write the changes that must be done to 'initialise.py'? FYI, as it is now, if I call |
or just make a Something lik this:
|
I checked the related issues/PRs and I am interested in working on a new init for colorama that doesn't have the known problems of the current one (I am referring to the final comment in #262 ). My terminal application was slowing down a lot and I found out it was due to the multiple init calls and no deinit (I wasn't able to find such information online), so I'd like to contribute to help others avoid these problems. I see the issues/PRs are still opened so I am assuming the feature hasn't been released yet, but I'd still like to ask to confirm. It's also been a couple of years so I'd like to make sure I am not missing anything. But so far my understanding is that the new init would run only once, and if the programmer inadvertently adds a second init, then it simply won't run. It will also not allow wrappers to restart values, for example Is that correct? |
@VelpaChallenger Be aware that there is already a new version of init, called If that doesn't handle your case, then I'm not sure what to recommend. Feel free to discuss it here, but don't produce a big PR because it probably will not get merged. We don't have a lot of time for changes to Colorama these days. Hugs! |
@tartley Yes, that one works. In fact, it also solves the problem of slowing down the terminal at all. Even with just one use, I don't see the slightest slow down. Very kind of you! Thanks for the response and the help 😄 . Hugs! |
I think
init()
should probably either raise an error, or no-op if colorama was already initialized.But in any case, it should not break
deinit
by losing the originalstdout
/stderr
.The text was updated successfully, but these errors were encountered: