Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding MultiBar logging #51

Open
bbugh opened this issue Feb 23, 2021 · 7 comments
Open

Adding MultiBar logging #51

bbugh opened this issue Feb 23, 2021 · 7 comments

Comments

@bbugh
Copy link

bbugh commented Feb 23, 2021

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%ff@email.com' (1912) updated
├── Thread 7 [= ] 50%on@email.com' (1911) updated
├── Thread 4 [= ] 50%tacos@gmail.com' (1913) updated
├── Thread 5 [= ] 50%super.com' (1915) updated
├── Thread 0 [= ] 50%ar@wow.com' (1608) updated
├── Thread 3 [= ] 50%oldgreg@nothing.com' (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
Copy link
Author

bbugh commented Feb 23, 2021

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

@bbugh
Copy link
Author

bbugh commented Feb 23, 2021

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 bbugh closed this as completed Feb 23, 2021
@piotrmurach
Copy link
Owner

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

@bbugh
Copy link
Author

bbugh commented Jun 2, 2021

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 bbugh reopened this Jun 2, 2021
@piotrmurach
Copy link
Owner

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.

@bbugh bbugh changed the title MultiBar logging is corrupted Adding MultiBar logging Jun 2, 2021
@bbugh
Copy link
Author

bbugh commented Jun 2, 2021

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 bbugh mentioned this issue Jun 4, 2021
5 tasks
@bbugh
Copy link
Author

bbugh commented Jun 4, 2021

I created #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants