tty-progressbar icon indicating copy to clipboard operation
tty-progressbar copied to clipboard

ProgressBar#current= supports indeterminate progress bars too

Open alexcwatt opened this issue 1 year ago • 4 comments

Describe the change

I noticed that #current= failed for indeterminate progress bars. The primary test I added would error like this:

     ArgumentError:
       comparison of NilClass with 5 failed

I added a new ArgumentError guard to also preserve the existing, desirable, behavior of not supporting #current = nil, and a test to cover this. Note that current = nil was already raising ArgumentError in both the determinate and indeterminate cases (comparison of Integer with nil failed, comparison of NilClass with 0 failed, etc.); now the error message is one that we set.

Why are we doing this?

As far as I can tell, there's no reason that current = shouldn't work for indeterminate progress bars. It seems that indeterminate progress bars are meant to have a current value, and can have it by calling .advance, so adding support seems consistent with the rest of the library.

Benefits

It's quite nice to be able to set current progress for indeterminate progress bars!

Drawbacks

I don't have any to highlight.

Requirements

  • [x] Tests written & passing locally?
  • [ ] Code style checked?
  • [x] Rebased with master branch?
  • [ ] Documentation updated?
  • [x] Changelog updated?

alexcwatt avatar Aug 23 '23 10:08 alexcwatt

@piotrmurach The errors now focus on "numeric" instead of a particular class. Let me know what you think!

alexcwatt avatar Sep 11 '23 15:09 alexcwatt

@piotrmurach I've made that small change for consistency

alexcwatt avatar Sep 11 '23 22:09 alexcwatt

@piotrmurach let me know if you want me to do anything else to make this mergeable

alexcwatt avatar Oct 23 '23 02:10 alexcwatt

@alexcwatt This is on my list, I promise I'm not ignoring you. Please bear with me.

piotrmurach avatar Nov 06 '23 22:11 piotrmurach