Handle nested progress bars #13
Conversation
|
This certainly looks promising to me 👍 |
|
This looks pretty good to me at a quick browse. I wondered if there should have a couple of extra tests here covering how the nesting is printed? |
| @@ -95,7 +95,7 @@ end | |||
|
|
|||
| function Base.push!(sticky::StickyMessages, message::Pair) | |||
| if !sticky.ansi_codes | |||
| write(sticky.io, message[2]) | |||
| println(sticky.io, rstrip(message[2])) | |||
There was a problem hiding this comment.
I tweaked how StickyMessages is handled in unsupported TTYs a bit. I think we need to ensure printing newline to be a bit more consistent with how it is handled in supported TTYs. Otherwise, multi-line sticky messages like nested progress bars are unreadable in such output streams. I wondered if I should use chomp here but I guessed it might be better to print only one newline to be as compact as possible.
|
OK, I added some tests for nested progress bars. Anything else you want to address here? |
test/TerminalLogger.jl
Outdated
| ("", (progress = "done", _id = 2222)), # 8 | ||
| ("Outer", (progress = 0.4, _id = 1111)), | ||
| ("", (progress = "done", _id = 1111)), | ||
| ]; width=60) |
There was a problem hiding this comment.
Just wondering whether you cover the case of inner progress with parent id's somewhere here?
There was a problem hiding this comment.
Ooooh, thanks a lot for noticing this. I totally forgot how it works 🤣. I added proper tests for nested progress bars now.
|
Thanks for the review! Do you think it's ready for another release? |
|
Sure, this is useful so if you want to release feel free. The prospect of a release has inspired me to finish the markdown hacking in #23 so we could try getting that in as well. |
This is an implementation of nested progress handling specified by RFC JuliaLogging/ProgressLogging.jl#13.
It's ready for review but I'm submitting it as a draft since it requires ProgressLogging.jl with JuliaLogging/ProgressLogging.jl#13 to be released.