Rockhopper-Technologies / enlighten

Enlighten Progress Bar for Python Console Apps
https://python-enlighten.readthedocs.io
Mozilla Public License 2.0
416 stars 25 forks source link

scroll isn't refreshed, in some circumstances #27

Closed davidhyman closed 4 years ago

davidhyman commented 4 years ago

Description In some circumstances, enlighten mangles the output from loggers (output disappears, becomes garbled, interspersed with progress bars).

This can be reproduced in the 'datacenters' demo by iterating another progress bar before closing the datacenter, or by using the repro code below.

expect:

INFO:root:welcome
INFO:root:ok!
INFO:root:ok?
INFO:root:ok??
INFO:root:ok!!

actual:

INFO:root:welcome
INFO:root:ok!

INFO:root:ok??

Investigation This took me a while to figure out because I thought it was something to do with the counter indices and their (lack of?) reassignment when removing counters. It sort of is, but only because when tracking the terminal cursor position it shows a difference between the two scenarios in the repro code below. This then seems to affect the scroll size.

To be specific; it appears that the issue is: https://github.com/Rockhopper-Technologies/enlighten/blob/master/enlighten/_manager.py#L336

if force or newOffset > oldOffset or newHeight != self.height:

the offsets check - this only changes the scroll size if the offset grew; however, the "broken" scenario in the repro code below causes the offset to shrink, but with no scroll resize occurring, it starts overwriting the log output stream. The following modification to that line fixes the issue:

if force or newOffset != oldOffset or newHeight != self.height:

It's possible this might not be the correct way to fix the issue - the choice of > might be handling some other case that I can't intuit; if so, the 'shrinking scroll area' may need some other explicit handling. Any thoughts most welcome!

To Reproduce This example demonstrates the problem. We expect to see all the log messages, but with 'broken' set True, we start getting corruption. In this simplest case, it appears as though the only difference is 'nesting' of progress bars - or their close order. From here I started to look at the counter position reassignments.

broken=False
import logging
import enlighten
logging.basicConfig(level=logging.INFO)
logging.info("welcome")
with enlighten.get_manager() as manager:
    with manager.counter(leave=False, desc="A") as a:
        a.refresh()
        if broken:
            with manager.counter(leave=False, desc="B") as b:
                b.update()
                with manager.counter(leave=False, desc="C") as c:
                    c.update()
        else:
            with manager.counter(leave=False, desc="B") as b:
                b.update()
            with manager.counter(leave=False, desc="C") as c:
                c.update()
        logging.info("ok!")
        bonus_bar = manager.counter(leave=False, total=3, desc="D")
        bonus_bar.refresh()
        bonus_bar.update()
        logging.info("ok?")
        bonus_bar.close()
        logging.info("ok??")
        logging.info("ok!!")

Environment

enlighten==1.6.0
avylove commented 4 years ago

Good find! And thanks for providing details! I'll have to dig into this, but I may not get a chance for a few days.

avylove commented 4 years ago

The problem turned out to be I wasn't preserving the maximum scroll offset. by design, Enlighten does not shrink the portion of the screen used for progress bars, but when the number of bars was reduced it wasn't preserving that size. The main reason not to reduce the area is for aesthetics. It's much easier on the eye than having things move too much.

Fix is in master. Should have a new release soon.

davidhyman commented 4 years ago

Took it for a spin and it looks great :sparkles:, thank you for your efforts :+1:

avylove commented 4 years ago

Thanks for trying it out! Released in 1.6.1