tty-progressbar icon indicating copy to clipboard operation
tty-progressbar copied to clipboard

Adding MultiBar logging

Open bbugh opened this issue 3 years ago • 7 comments

Hi! 👋 Firstly, THANK YOU FOR THESE AMAZING GEMS! The TTY suite is exceptional.

Describe the problem

When using TTY::ProgressBar::Multi, logging with each registered bar results in corrupted output.

Steps to reproduce the problem

multi_bar = TTY::ProgressBar::Multi.new("Example [:bar] :percent", total: examples.count)

bars = [
    multi_bar.register("Thread 1 [:bar] :percent", total: examples.count / 4),
    multi_bar.register("Thread 2 [:bar] :percent", total: examples.count / 4),
    multi_bar.register("Thread 3 [:bar] :percent", total: examples.count / 4),
    multi_bar.register("Thread 4 [:bar] :percent", total: examples.count / 4)
]

Parallel.each(examples) do 
    bars[Parallel.worker_number].log "Something"
end

Actual behaviour

The previous logs are overwritten:

├── Thread 6 [= ] 50%[email protected]' (1912) updated
├── Thread 7 [= ] 50%[email protected]' (1911) updated
├── Thread 4 [= ] 50%[email protected]' (1913) updated
├── Thread 5 [= ] 50%super.com' (1915) updated
├── Thread 0 [= ] 50%[email protected]' (1608) updated
├── Thread 3 [= ] 50%[email protected]' (1874) updateded

Expected behaviour

Logging is printed above the multibar

Describe your environment

  • OS version: Mac OS Mojave
  • Ruby version: 2.6.6 something
  • TTY::ProgressBar version: 0.18.0

bbugh avatar Feb 23 '21 18:02 bbugh

If it's easier, TTY::ProgressBar::Multi could have a log method rather than each individual bar logging, which would be fine too.

bbugh avatar Feb 23 '21 18:02 bbugh

Sorry, this seems to be an issue with using Parallel more than this gem. It looks like Parallel would have to integrate with this for it to be successful.

bbugh avatar Feb 23 '21 18:02 bbugh

Hi Brian 👋

Thanks for using tty-progressbar.

Currently, the TTY::ProgressBar::Multi API doesn't have log support. I'd consider adding it if you have time to send a pull request. The behaviour should be similar to TTY::ProgressBar#log in so far as the logging would be printed above all registered bars.

Not sure if this helps but you can update an individual bar display with custom tokens using the advance method like so:

Parallel.each(examples) do 
  bars[Parallel.worker_number].advance(X, name: "Something")
end

piotrmurach avatar Feb 26 '21 20:02 piotrmurach

Hey there, I'm having a crack at this again because I have another case where I need multiple processes and need to log from them. I've tried lots of different things to add log to ProgressBar::Multi and they all seem to have an edge case. I tried using TTY::Cursor.up and TTY::Cursor.down to reposition, along with flushing the output, NEXTLINE, etc. The best I can come up with is overwriting the top line.

I think one of the root problems is that since the current row is determined in the bar when it prints, it seems like there's not a reliable way to use up and down from the multi bar. Another is that move_to_row overrides anything that might be going on in the multi progress bar if writing via a bar.

Do you have any hints for how I could make this work?

bbugh avatar Jun 02 '21 19:06 bbugh

I feel the title of this issue is a bit misleading. The log method is only part of the TTY::ProgressBar API and doesn't mention any support for multi-level progress bar logging. I think it would be better to rename it as a feature request?

I haven't thought about how the log method could work for TTY::Progress::Multi. It seems that one alternative approach to making this work could be to make the individual progress bar log method aware of multibar context. Since the current bar knows its position within the multi progressbar, you have a reliable way to print above all the bars the same way move_to_row does it. Then the log implementation on the Multi would mean delegating logging via all progress bars or picking an individual bar. This could work? Please submit PR if you have anything working as it's much easier for me to see where potential issues are.

piotrmurach avatar Jun 02 '21 21:06 piotrmurach

I fixed the title, thanks for catching that.

I tried for a while to log via top_bar and that was the closest to success I had, but I couldn't get it to log a line, and then go to the next line, and then start drawing the bars again. It would draw over the top line instead.

I will make a draft PR tonight and go from there. Thanks!

bbugh avatar Jun 02 '21 21:06 bbugh

I created https://github.com/piotrmurach/tty-progressbar/pull/52 that has WIP code for this. Two examples of getting close but not quite.

There's a multi_test.rb in the root folder that you can run to quickly test, if you want. (I will of course delete this and write real tests once it's figured out)

Thanks for taking a look.

bbugh avatar Jun 04 '21 23:06 bbugh