search

nathanpeck / clui

Command Line UI toolkit for Node.js
MIT License
1.66k stars 40 forks source link

update Progress Bar, define maxValue at initialization #5

Closed jackchoon closed 7 years ago

nathanpeck commented 9 years ago

Thanks for providing this. However, I can not merge this in as is because it completely removes the ability to update the max value dynamically.

In some workloads the total amount of work can not be accurately determined in advance, and it is necessary to update the max value on the fly, allowing the progress bar to skip ahead or move backwards as the workload is either expanded or decreased.

I think an acceptable solution would be to allow the user to setup a predefined max value on the constructor if they wish and then optionally leave out the max value when calling ProgressBar.update(), but also retain the ability to change the max value in the ProgressBar.update() method if needed. If you update your pull request with this change I will definitely merge it in.

Thanks!

jackchoon commented 9 years ago

Thanks for considering and suggestion. I will make an improvement on it. Do you think a 'percentage' is acceptable when update ? 

ProgressBar.update( percent )
artokun commented 7 years ago

Sounds like, if there is a single argument in update() then it will be percent based. If two arguments then it will revert to.update(currentValue, maxValue) @nathanpeck @jackchoon

artokun commented 7 years ago

This functionality is now available in v0.3.6 .update(currentValue, maxValue) and .update(percent)