EvanHahn / HumanizeDuration.js

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

enabling round does not give correct result #98

Closed tazmaniax closed 8 years ago

tazmaniax commented 8 years ago

Hi,

If I use humanizeDuration(moment().diff(thisDate), { delimiter: ' ', largest: 3 }); then I get something like 21 minutes 16.291 seconds but if I include the round parameter like humanizeDuration(moment().diff(thisDate), { delimiter: ' ', largest: 3, round: true }); then I get 0 seconds

I'm using v3.7. Am I doing something wrong?

thx, Chris

EvanHahn commented 8 years ago

Hmm. Could you provide a sample input (ie, what is the value of moment().diff(thisDate))?

tazmaniax commented 8 years ago

thisDate is "2016-04-16T22:13:56+0100" and the duration coming out of moment().diff(thisDate) is 2838550.

EvanHahn commented 8 years ago

Hmm, this looks right to me:

humanizeDuration(2838550, { delimiter: ' ', largest: 3 })
// => "47 minutes 18.55 seconds"

humanizeDuration(2838550, { delimiter: ' ', largest: 3, round: true })
// => "47 minutes 19 seconds"

Is that the output you're getting?

andrewwylde commented 8 years ago

I'm getting the same result as @tazmaniax . Looks like it's an issue around the rounding issue.

humanizeDuration( 2588982, { largest: 1, round: true } );

Once it enters the if (options.round) block, pieces looks as thus:

image

But it comes out image

and eventually sets them all to 0.

it appears to be looping through and setting the value to 0 ~ line 373:

if ((piece.unitCount % ratioToLargerUnit) === 0 || (options.largest && ((options.largest - 1) < i))) {
    previousPiece.unitCount += piece.unitCount / ratioToLargerUnit
    piece.unitCount = 0
}

Might need a bit more digging to figure out exactly what's going on, but I at least narrowed it down to that.

EvanHahn commented 8 years ago

Thanks for reporting and digging--I think I may have been testing on an old version. I'll take a look.

EvanHahn commented 8 years ago

Sorry I'm slow to fix this! I've been busy.

EvanHahn commented 8 years ago

This has been fixed in humanize-duration@3.7.1. Thanks to @brandly for his helpful PR!

tazmaniax commented 8 years ago

Thx!