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

Added support for multiple progress bars displayed simultaneously #69

Merged
merged 11 commits into from
Apr 24, 2020

Conversation

vehovsky
Copy link
Contributor

@vehovsky vehovsky commented Apr 7, 2020

  • defaults to "single line" behavior when terminal does not support moving cursor
  • using ScheduledThreadPoolExecutor with single daemon thread (that does not prevent the JVM from exiting)
    • simplifies thread-safety and ensures only single thread is moving cursor

Fixes #11

mordechaim and others added 6 commits January 28, 2020 20:08
…arted from multiple threads / concurrently)

 - defaults to "single line" behavior when terminal does not support moving cursor
 - using ScheduledThreadPoolExecutor with single daemon thread (that does not prevent the JVM from exiting)
   - simplifies thread-safety and ensures only single thread is moving cursor
 - moving cursor up/down only based on the fact how many active progress bars are currently being rendered (instead of getting current cursor position in order to attempt to "re-render" correct line in console - which breaks instantly on terminal re-size)
@vehovsky
Copy link
Contributor Author

vehovsky commented Apr 7, 2020

I've tested on

Windows 10:

  • Command Prompt (works)
  • Git Bash - MSYS2 (works)
  • Cygwin (does not work)

MacOS:

  • Terminal (works)
  • iTerm (works)

Centos7:

  • trough PuTTY (works)

@ctongfei
Copy link
Owner

ctongfei commented Apr 7, 2020

Wow! Thanks @vehovsky !!!
I'll review this asap (very busy right now, so I apologize for any delay) and push a new version.

@ctongfei ctongfei added this to the 0.9 milestone Apr 7, 2020
@vehovsky
Copy link
Contributor Author

vehovsky commented Apr 8, 2020

I've used this simple code to verify on the different envs: progressbas-multiline-example
Be aware: to run it you need to "mvn install" changes from this PR (also change version to 0.8.2)

 - no need for entering terminal raw mode (needed when asking terminal for cursor position)
 - basic terminal info (width / cursor movement support) are now public - helps when writing custom ProgressBarConsumer
@@ -123,7 +123,7 @@ public ProgressBar(
@Deprecated
public ProgressBar start() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't see point in keeping this deprecated method in the public API. There is no way anymore (even before this PR) to construct the ProgressBar without the Runnable being started. Personally I would remove it and made the updateInterval private again in ProgressThread.

…e consumer

  - currently was not the case and could cause display issue in terminals without cursor movement support (when multiple progress bars were started in parallel)
    - as the final "new line" was being writen to PrintStream from different thread
- other refactorings
 - ensure new terminal is not created on every progress bar re-render
 - at the same time ensure only single instance of Terminal was build
   - every ProgressBar can't hold instance of Terminal, although it's possible to create multiple instances of Terminal, subsequent instance are created as "dumb" until the first instance is closed
@ctongfei ctongfei changed the base branch from master to 1.0 April 22, 2020 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When used in multiple thread, all the progress bars are in one line
3 participants