fix: display decimals on largest unit #171

HHK1 commented 4 years ago

Fixes https://github.com/EvanHahn/HumanizeDuration.js/issues/150

As explained in the issue, using the library with largest set (with or without units specified), results in leaving the decimal part for units larger than second.

This fixes this behaviour and adds unit tests
I changed the way rounding was working since it leads to inaccurate results
Added some tests regarding: https://github.com/EvanHahn/HumanizeDuration.js/issues/141 since it was linked to the issue above. I don't see any problems there though.

Now, some tests are breaking, specifically:

const h = humanizer({ largest: 2 })
assert.strictEqual(h(540360012), '6 days, 6 hours')
assert.strictEqual(h(540360012, { largest: 3 }), '6 days, 6 hours, 6 minutes')

I haven't touched those to avoid hiding this. I think it's reasonable to have those tests failed if round or maxDecimalPoints is not provided, the only issue is that it's going to be a "breaking" change in those cases.

You can leave it as is (and fix the test ofc), and publish a major version, or only apply the remainder if maxDecimalPoints has been provided, what do you think ?

HHK1 commented 4 years ago

@EvanHahn Re-opened after the auto-close due to main branch renaming. Still unclear on what you would like me to do with it though, should I point to your v4 feature branch instead ?

EvanHahn commented 4 years ago

Sorry that I'm slow to respond here. This is on my list of things to do, but admittedly low the list.

EvanHahn commented 1 year ago

It's been a few years but I'm finally getting around to this.

Could you try this on the latest version of humanize-duration to see if this is still a problem? I think I may have fixed it.

EvanHahn commented 7 months ago

I'm going to close this because it has been open for several years with no activity. Please let me know if that's wrong and we can reopen.

EvanHahn / HumanizeDuration.js

fix: display decimals on largest unit #171 #191