MultiProgress: prune 'zombie' progress bars; fixes #426; add render tests

[chris-laplante]

No description provided.

[chris-laplante]

@djc FYI this draft PR so far just introduces a test that demonstrates issue #426. I am working on implementing the fix.

[djc]

This looks pretty sensible, commented on some possible improvements.

[djc]

So with draw_state: Option<_> and is_zombie: bool we have 4 different states. Do we need all 4? Maybe that would be a useful thing to explain in a comment?

I was surprised Weak implements Default -- it clearly works but it feels a little ugly to me. If we can collapse some of the states maybe there's something better we can do?

[chris-laplante]

Well, only three states are necessary:

1. On MultiProgress::internalize (and before draw) draw_state will be None and is_zombie will be false.
2. On draw, draw_state will be Some(..) and is_zombie will be false.
3. When a zombie is detected, is_zombie will become true. Normally draw_state will be Some(..) but I suppose it's possible for a pb to be briefly in zombie state if it was never drawn (i.e. you immediately drop it after adding to the MultiProgress).

I'll add a comment to this effect.

[chris-laplante]

One option would be to have an enum, but I think that would make the code even uglier.

[djc]

By "suppose it's possible", I think you're suggesting that we actually do have 4 states?

[chris-laplante]

Yes, there are technically four states. But state 3 and 4 (with 4 being "is_zombie == true" and "draw_state is None") are handled the same.

[chris-laplante]

OK, this should be a little better (hopefully)

[djc]

Would you mind squashing the review feedback commit back into the originating commit? Would make reviewing easier.

[chris-laplante]

OK, squashed. Sorry about that. I know I need to get better at structuring my commits :/