refactor(cli/unstable): make `ProgressBar` `options` optional

[timreichen]

This PR make ProgressBar options an optional argument. This is done by setting options.max to 1 by default. This default value is taken from HTMLProgressElement.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 94.54%. Comparing base (390845e) to head (5395e07). :warning: Report is 163 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6407   +/-   ##
=======================================
  Coverage   94.54%   94.54%           
=======================================
  Files         580      580           
  Lines       43730    43731    +1     
  Branches     6471     6471           
=======================================
+ Hits        41345    41346    +1     
  Misses       2342     2342           
  Partials       43       43           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[BlackAsLight]

I'm not a fan of this change as, I imagine you're always going to want to set a max value, and forgetting to set one where the value goes over max will throw an error

[timreichen]

I'm not a fan of this change as, I imagine you're always going to want to set a max value, and forgetting to set one where the value goes over max will throw an error

I don't think value > max should throw. This might lead to unintended bugs with floating-point errors. So we might want to change that which would pave the way for a default max (as in HTMLProgressElement).

WDYT?

[BlackAsLight]

I'm not a fan of this change as, I imagine you're always going to want to set a max value, and forgetting to set one where the value goes over max will throw an error

I don't think value > max should throw. This might lead to unintended bugs with floating-point errors.

So we might want to change that which would pave the way for a default max (as in HTMLProgressElement).

WDYT?

It will throw when the maths has "-".repeat(-1) in the print function. As it doesn't seem to like negative numbers. It could be wrapped in a Math.max to stop this with the end result be that the progress bar just grows past its bar length.

[timreichen]

I'm not a fan of this change as, I imagine you're always going to want to set a max value, and forgetting to set one where the value goes over max will throw an error

I don't think value > max should throw. This might lead to unintended bugs with floating-point errors. So we might want to change that which would pave the way for a default max (as in HTMLProgressElement). WDYT?

It will throw when the maths has "-".repeat(-1) in the print function. As it doesn't seem to like negative numbers. It could be wrapped in a Math.max to stop this with the end result be that the progress bar just grows past its bar length.

I'll have a look at that. clamping these values seems like a good idea anyway.

[kt3k]

Because this module formats the value as byte numbers by default, setting max=1 as default feels strange to me.

I'm not a fan of this change as, I imagine you're always going to want to set a max value

If we consider max as required argument, then we probably should set max as positional argument instead of option

[timreichen]

Because this module formats the value as byte numbers by default, setting max=1 as default feels strange to me.

I'm not a fan of this change as, I imagine you're always going to want to set a max value

If we consider max as required argument, then we probably should set max as positional argument instead of option

I was thinking about this a bit. max = 1 just seems right for a progress indication as a default. This causes a problem now because the current implementation is geared towards data/downloads which is not the only progress indication type. So I was wondering: Should we split up the ProgressBar and the ProgressBarStream implementation, making the ProgressBar sync like Spinner and remove the unit implementation and indicate percentages instead and let ProgressBarStream work like it does now for a data driven indication?

Another option would be to remove the custom fmt property from ProgressBar and instead introduced a style property that determine the format. Something like:

type ProgressBarType = "percentage" | "download"
new ProgressBar({ type: "percentage" }) // [###-------] [30%]
new ProgressBar({ type: "download" }) // [mm:ss] [###-------] [0.40/97.66 KiB]

This would be less customizable but simplify the api. I am aware that I proposed the format property instead of style options.

[timreichen]

stale