EvanHahn / HumanizeDuration.js

361000 becomes "6 minutes, 1 second"
https://evanhahn.github.io/HumanizeDuration.js/
The Unlicense
1.64k stars 175 forks source link

Fix round with largest #100

Closed brandly closed 8 years ago

brandly commented 8 years ago

I saw #98 and figured it'd be a good excuse to dig around and see how this project actually works.

As @astoellis pointed out, the problem occurred when you use both the round and largest options at once. That block of code he mentions only gets hit when both options are active.

Slightly farther down, the largest option is handled properly, by waiting for largest number of pieces that actually have a value before breaking out of there.

However, the problematic section of code only relates the largest value to the current i, with no regards to which pieces are actually occupied by a non-zero unitCount. In effect, this means that round assumes that a largest: 3 option refers to units years/months/weeks. If the initial ms you're calculating is less than a week, you're left with all zeros, resulting in the 0 seconds output.

By calculating firstOccupiedUnitIndex up front, we can properly compare the current index to largest when rounding.

EvanHahn commented 8 years ago

@brandly You're my hero