Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isoDate parser cannot cope with ISO-8601 combined dates and times #247

Closed
exterrestris opened this issue Feb 25, 2013 · 9 comments
Closed

Comments

@exterrestris
Copy link

The regex used by the isoDate parser will match an ISO-8601 date/time such as 2013-02-18T18:18:44+00:00, but cannot parse such a date - it instead returns an empty string.

I have worked around this by disabling the isoDate parser, and using my own - it cannot parse non-GMT times, but this is sufficient for my needs. I believe the date.js library can parse ISO-8601 dates successfully however.

var iso8601date = /^([0-9]{4})(-([0-9]{2})(-([0-9]{2})(T([0-9]{2}):([0-9]{2})(:([0-9]{2})(\.([0-9]+))?)?(Z|(([-+])([0-9]{2}):([0-9]{2})))?)?)?)?$/;

$.tablesorter.addParser({
// set a unique id
id : 'iso8601date',
is : function(s) {
    return s.match(iso8601date);
},
format : function(s) {
    var result = s.match(iso8601date);

    if (result) {
    var date = new Date(result[1], 0, 1);

    if (result[3]) {date.setMonth(result[3] - 1);}
    if (result[5]) {date.setDate(result[5]);}
    if (result[7]) {date.setHours(result[7]);}
    if (result[8]) {date.setMinutes(result[8]);}
    if (result[10]) {date.setSeconds(result[10]);}
    if (result[12]) {date.setMilliseconds(Number('0.' + result[12]) * 1000);}

    return date;
    }

    return 0;
},
type   : 'numeric'
});
@Mottie
Copy link
Owner

Mottie commented Feb 25, 2013

Hi seanellingham!

Thanks for reporting this! I figured out a very simple fix for this... just change the built-in parser to this:

ts.addParser({
    id: "isoDate",
    is: function(s) {
        return (/^\d{4}[\/\-]\d{1,2}[\/\-]\d{1,2}/).test(s);
    },
    format: function(s, table) {
        return ts.formatFloat((s !== "") ? (new Date(s).getTime() || "") : "", table);
    },
    type: "numeric"
});

All I needed to do was remove the .replace(/-/g, "/") inside of the new Date() function.

I'll have this added in the next update. Thanks!

@Mottie
Copy link
Owner

Mottie commented Feb 25, 2013

Hmm, ok I just discovered that IE, doesn't like parsing dates with dashes. I'll try to figure something out.

@Mottie
Copy link
Owner

Mottie commented Feb 25, 2013

So after some deliberation, I think isoDate is the appropriate name for this parser; it will parse time, but really as an afterthought. So, I don't think I'm going to change it.

But, I do think your parser works great and I'll include it with a bunch of other parsers in version 3.

Also, I believe a more general method would be best. And since many great date libraries are already written, I wrote this parser to use Sugar (demo):

$.tablesorter.addParser({
    id: "sugarDate",
    is: function(s) {
        // we're not detecting because of the range of values
        return false;
    },
    format: function(s) {
        return Date.create(s).getTime() || s;
    },
    type: "numeric"
});

@matthewslaney
Copy link

Hi Mottie,

Thanks for tablesorter, it's great. I was running into problems with ISO 8601 sorting as well and found this bug. However, your update didn't actually resolve it for me. Here's the updated parser that I created that works for me.

    ts.addParser({
        id: "isoDate",
        is: function(s) {
            return (/^\d{4}[\/\-]\d{1,2}[\/\-]\d{1,2}T\d{2}:\d{2}:\d{2}[\+,\-]\d{2}:\d{2}/).test(s);
        },
        format: function(s, table) {
            return ts.formatFloat((s !== "") ? (new Date(s).getTime() || "") : "", table);
        },
        type: "numeric"
    });

@Mottie
Copy link
Owner

Mottie commented Mar 13, 2016

Hi @matthewslaney!

The "isoDate" parser in the latest version makes sure that a valid date is obtained, so it now looks like this:

ts.addParser({
    id : 'isoDate',
    is : function( str ) {
        return /^\d{4}[\/\-]\d{1,2}[\/\-]\d{1,2}/.test( str );
    },
    format : function( str, table ) {
        var date = str ? new Date( str.replace( /-/g, '/' ) ) : str;
        return date instanceof Date && isFinite( date ) ? date.getTime() : str;
    },
    type : 'numeric'
});

@matthewslaney
Copy link

@Mottie,

Ah. That makes sense. The only part I was updating was the regex. The version I gave will now properly match the true ISO 8601 format: 2013-02-18T18:18:44+00:00.

@Mottie
Copy link
Owner

Mottie commented Mar 13, 2016

Well the "iso8601date" parser is separate from the built-in "isoDate" parser. I left off the time portion of the "isoDate" parser intentionally since it was meant to be a bit more generic because not everyone includes a time.

@matthewslaney
Copy link

I see. I guess my confusion is that out of the box a table with the full date and time is not properly parsed. I was just trying to contribute a quick fix since I found this project so useful.

@Mottie
Copy link
Owner

Mottie commented Mar 14, 2016

Hmm, I think you're right... the default "shortDate" parser would catch any basic formats. Maybe I should replace the "isoDate" with and "iso8601date" parser.... or remove it from the core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants