jsmreese / moment-duration-format

Format function plugin for the Moment Duration object.
MIT License
968 stars 120 forks source link

milliseconds are always three digit #54

Closed codeofsumit closed 6 years ago

codeofsumit commented 8 years ago

I'm desperately trying to get the following format: mm:ss:SS while SS is double digit milliseconds.

This is the code i use:

var formattedTime = moment.duration(ms).format('mm:ss:SS', {
    trim: false,
    precision: 0
});

this is before I start counting up the ms variable (as in, starting the timer): image It's correct.

This is after I start the timer: image This is not correct.

I add 10 to ms every 10 milliseconds via an interval.

Any ideas what I might be doing wrong?

jsmreese commented 8 years ago

Huh. Interesting issue. Easily testable independently with this code:

(function () {
    var i;
    for (i = 1000000; i < 1100000; i += 1047) {
        console.log(i, moment.duration(i).format('mm:ss:SS', {
            trim: false,
            precision: 0
        }));
    }
})();

Sure enough, you get three-digit milliseconds output when the milliseconds are greater than two digits. I guess I never considered the case where you'd want to format milliseconds this way?

You can work around this by using the precision argument, and doing a replace on the formatted string to get the separator you want (replacing whatever decimal separator you use):

(function () {
    var i;
    for (i = 1000000; i < 1100000; i += 1047) {
        console.log(i, moment.duration(i).format('mm:ss', {
            trim: false,
            precision: 2
        }).replace('.', ':'));
    }
})();
codeofsumit commented 8 years ago

thanks yeah, I used that workaround up until now and will continue to do so.

If you have a timer for, let's say a time based game, then (I guess) you'd want two digit milliseconds for the timer.

jsmreese commented 8 years ago

Also, if you're using the version from my dev branch, you can set a custom decimal separator:

(function () {
    var i;
    for (i = 1000000; i < 1100000; i += 1047) {
        console.log(i, moment.duration(i).format('mm:ss', {
            trim: false,
            precision: 2,
            decimalSeparator: ':'
        }));
    }
})();
arnaud-soulard commented 7 years ago

I encountered the same issue. It seems that moment.duration always returns milliseconds on 3 digits. I added a fix in moment-duration-format.js (line 369):

if (token.length > 1 && (foundFirst || token.isMost || settings.forceLength)) { if (token.type === 'milliseconds') { val = parseInt(token.value.toString().substr(0, token.length)); } val = padZero(val, token.length); }

It does the job, and I am now able to format moment.duration with 1, 2 or 3 digits for milliseconds. What do you think of the bug fix?

jsmreese commented 6 years ago

I'll take a look at this. The new version 2 of the plugin (currently on master but not yet published) removes the decimalSeparator option in favor of locale-based formatting via toLocaleString.

@arnaud-soulard your solution looks good at first glance.

jsmreese commented 6 years ago

I've implemented special handling for MS token length of 2. That's now on master. MS token length of 3 seems like it will naturally work out. A forced MS token length of 1 seems unlikely for this use case, and problematic to handle gracefully based on how every other token works.