colorama icon indicating copy to clipboard operation
colorama copied to clipboard

deinit() doesn't work if init() was called twice.

Open gilch opened this issue 6 years ago • 19 comments

>>> import colorama
>>> import sys
>>> id(sys.stdout)
2198406407512
>>> colorama.init()
>>> id(sys.stdout)
2198419631912
>>> colorama.deinit()
>>> id(sys.stdout)  # back to the original, as expected
2198406407512
>>> colorama.init()
>>> colorama.init()
>>> colorama.deinit()
>>> id(sys.stdout)  # What?
2198419494224

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 original stdout/stderr.

gilch avatar Sep 26 '17 00:09 gilch

I oringinally thought that this problem could be solved by just running deinit() again (as init() was called twice therefore to go back would require two deinit() calls) to get back to the original stdout.

However this does not seem to be the case when I started investigating, I found out that the second deinit() does not have any affect on the output of stdout, however the first deinit() seems to have some effect and lets you go back to the stdout output after the first init() was called.

>>> 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 init() are made without calling deinit(). Calling init() 3 times in a row and then calling deinit() 3 times makes stdout print out the same result as after the first init() this implies that multiple init() calls have an effect on stderr while multiple deinit() calls have no effect on stderr after the first deinit() call was made.

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.

joshuawise610 avatar Dec 05 '17 17:12 joshuawise610

If memory serves, init() uses static variables to store the original stdout/stderr, so calling it twice really does overwrite and lose the original.

tartley avatar Feb 21 '18 17:02 tartley

I'm worried about calling init(), someone could already called it before. So I want to fix this issue.

Should reinit() be dropped or deprecated?

pslacerda avatar Jun 14 '18 18:06 pslacerda

To @gilch, Sep 26, 2017

  1. I have checked id(sys.stdout) before init() and after deinit() and indeed, they are the same as in your code. Yet, regular printing is not restored. The text colors and style set with the last 'colorama' call still work!
  2. So, what is your solution for making deinit() work?

AlkisPis avatar Nov 10 '19 09:11 AlkisPis

@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

tartley avatar Nov 11 '19 03:11 tartley

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.

tartley avatar Nov 11 '19 13:11 tartley

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()'.

tartley avatar Nov 11 '19 13:11 tartley

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 stdout and keep a reference on stdout.__colorama_original_stdout. Init and deinit would call hasattr(stdout, '__colorama_original_stdout') it being lazy initialized or explicitly. Seems that it would avoid recursion errors.

pslacerda avatar Nov 11 '19 14:11 pslacerda

Yuck, this code I wrote is rubbish. Sorry people. :-) Upvotes on this comment are acceptable.

tartley avatar Nov 11 '19 15:11 tartley

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...

tartley avatar Nov 11 '19 15:11 tartley

Can someone with a Windows machine try out the branch above? Run tests and review the changes? Many thanks.

tartley avatar Nov 11 '19 16:11 tartley

For me seems alright but no Windows around here either.

pslacerda avatar Nov 11 '19 16:11 pslacerda

@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.

Why am I not playing nice? Anyway, this is an opportunity for me to thank you for your nice package! :)

AlkisPis avatar Nov 12 '19 10:11 AlkisPis

Can someone with a Windows machine try out the branch above? Run tests and review the changes? Many thanks.

@tartley, I have a Windows OS. What "branch" exactly to test? I visited https://github.com/tartley/colorama/pull/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 init(() once and then deinit(), the initial stdout ID is restored. But if I call init(() twice, then whatever number of times I call deinit(), I get the ID I got after the first call to init(), never the initial ID (before the first call to init()).

AlkisPis avatar Nov 12 '19 20:11 AlkisPis

I oringinally thought that this problem could be solved by just running deinit() again (as init() was called twice therefore to go back would require two deinit() calls) to get back to the original stdout.

However this does not seem to be the case when I started investigating, I found out that the second deinit() does not have any affect on the output of stdout, however the first deinit() seems to have some effect and lets you go back to the stdout output after the first init() was called.

>>> 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 init() are made without calling deinit(). Calling init() 3 times in a row and then calling deinit() 3 times makes stdout print out the same result as after the first init() this implies that multiple init() calls have an effect on stderr while multiple deinit() calls have no effect on stderr after the first deinit() call was made.

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.

or just make a .deinit() call in .init() definition

Something lik this:

def init(*agrs, **kwargs) :
    deinit(*propper_args, **propper_kwargs)
    '''following
    init
    method
    implementation'''

SamuelJansen avatar Jan 04 '21 14:01 SamuelJansen

@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'?

You test using the branch "multiple-init"

TheTechRobo avatar Aug 25 '21 14:08 TheTechRobo