feat: proposal live update progressbar

[KarolosLykos]

Description

I thought it would be great to have real time updates on the progressbarPrinter.

Scope

Until, the new concurrency pattern is ready #439 I used the same one with spinnerPrinter. These two printers are very similar and the probably will be refactored to prevent data races #482 in the future.

• Bug Fix
• New Feature
• Documentation
• Other

Related Issue

Fixes #460

To-Do Checklist

• I tested my changes
• I have commented every method that I created/changed
• I updated the examples to fit with my changes
• I have added tests for my newly created methods

[KarolosLykos]

@MarvinJWendt what do you think is that something that would make sense?

[MarvinJWendt]

Hi, I am not quite sure if I understand your intent correctly. Why would we need live updates? Currently, the progress bar should be updated, as soon as something changes. With a fixed delay for updates, we would re-render the progress bar, even if nothing changed.
Or is this for the elapsed time?

[KarolosLykos]

Hi, I am not quite sure if I understand your intent correctly. Why would we need live updates? Currently, the progress bar should be updated, as soon as something changes. With a fixed delay for updates, we would re-render the progress bar, even if nothing changed. Or is this for the elapsed time?

This is for the elapsed time. I saw the issue #460 and I thought I would give it a try.

[MarvinJWendt]

Ah, my bad. Yes, this would be a way to fix that.
But we should only update on a fixed time interval when ShowElapsedTime is true, so that progress bars that don't show the time, aren't updated unnecessarily. Can you change that?

[codecov]

Codecov Report

Merging #486 (1d4fac6) into master (ff7c49b) will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
+ Coverage   82.78%   82.89%   +0.11%     
==========================================
  Files          30       30              
  Lines        3467     3473       +6     
==========================================
+ Hits         2870     2879       +9     
+ Misses        548      546       -2     
+ Partials       49       48       -1     

Impacted Files	Coverage Δ
progressbar_printer.go	100.00% <ø> (+1.80%)	⬆️

[KarolosLykos]

Ah, my bad. Yes, this would be a way to fix that. But we should only update on a fixed time interval when ShowElapsedTime is true, so that progress bars that don't show the time, aren't updated unnecessarily. Can you change that?

Yes, that make sense, I added a flag so when the ShowElapsedTime is off the progress bar is printed only once. When, the counter is updated the progress bar is updated.

Also, can you check why I have so big difference on the coverage. I didn't change so much and I tested all my changes so at least I was expecting to be equal.

[howieyuen]

I have clone the feature branch and test with example code in issue #460, and I found the elapsed time is accumulated from the first task, shall we print the elapse time for every job ?

[KarolosLykos]

@howieyuen The progress bar is meant to give an overview of the overall progress and elapsed time of all the tasks combined. It's not necessary to print the elapsed time for each individual job since it may not be very informative and can clutter the output. So, let's stick to the original progress bar as a high-level overview of the job progress.

[howieyuen]

@howieyuen The progress bar is meant to give an overview of the overall progress and elapsed time of all the tasks combined. It's not necessary to print the elapsed time for each individual job since it may not be very informative and can clutter the output. So, let's stick to the original progress bar as a high-level overview of the job progress.

ok, thanks for your reply

[KarolosLykos]

@MarvinJWendt Can you review and let me know your thoughts on the changes?

[MarvinJWendt]

Hi @KarolosLykos, I tried the implementation in IntelliJ (Goland) and the default Windows Terminal.

Current implementation

This PR

Windows Terminal

It seems like the terminals cannot handle so many terminal updates easily and the progress bar starts flickering.

I also tried this PR with a delay of 1 second, but that (somehow) made the flickering even worse.

This is definitely a problem with the terminals and not with this PR, but we have to consider that PTerm must work on most terminals.

Do you have an idea, how this could be fixed?

[KarolosLykos]

Hi @KarolosLykos, I tried the implementation in IntelliJ (Goland) and the default Windows Terminal.

Current implementation

This PR

Windows Terminal

It seems like the terminals cannot handle so many terminal updates easily and the progress bar starts flickering.

I also tried this PR with a delay of 1 second, but that (somehow) made the flickering even worse.

This is definitely a problem with the terminals and not with this PR, but we have to consider that PTerm must work on most terminals.

Do you have an idea, how this could be fixed?

Sorry for the late response, I understand your point of view. However, in my opinion, flickering may be unavoidable, especially when the terminal emulator's latency is high. Also, since we are testing with dummy data and not actual time-consuming tasks, the progress bar gets updated frequently, leading to more flickering. An improved approach to handling things like this, could be a push-based solution instead of fixed interval polling.

[howieyuen]

any update on this ?

[MarvinJWendt]

Hi, this is now fixed in #500. I don't know why this PR caused flickering, but in #500 it works consistently. Maybe it's because of the other improvements in #500.

It seems, that the flickering was not caused by frequent updates, but by something else. I have tested #500 with 100 updates / second and there is no flickering at all.

Thanks to both of you @KarolosLykos and @howieyuen for helping with this!

@KarolosLykos: I will give you credits in the release, as a compensation for closing this PR