Skip to content

Timezone fix#20

Closed
MartinNuc wants to merge 2 commits into
eight04:masterfrom
MartinNuc:timezone-fix
Closed

Timezone fix#20
MartinNuc wants to merge 2 commits into
eight04:masterfrom
MartinNuc:timezone-fix

Conversation

@MartinNuc

Copy link
Copy Markdown
Contributor

Tried to use this directive with datetime-model="yyyy-MM-ddTHH:mm:ss.sssZ" but it failed with undefined reference error. Also format of timezone offset is expected without :. According to https://www.ietf.org/rfc/rfc3339.txt there should be : sign. I found that only angular date filter uses format without :.

@eight04

eight04 commented Mar 29, 2016

Copy link
Copy Markdown
Owner

941e02f Looks good.

But the ":" sign is optional according to:

time-numoffset = ("+" / "-") time-hour [[":"] time-minute]

@eight04

eight04 commented Mar 30, 2016

Copy link
Copy Markdown
Owner

Merged 941e02f and release 2.2.1.

@eight04 eight04 closed this Mar 30, 2016
@MartinNuc

Copy link
Copy Markdown
Contributor Author

I will test further possible cases with : sign. In my commit : (? in regex) is optional but there is some checking conversion after which results in a different string than input and throws an exception. I will try to think how to solve it.

Thanks for merging undefined fix.

@eight04

eight04 commented Mar 31, 2016

Copy link
Copy Markdown
Owner

If you are getting this error while parsing, you can use err.properText:

try {
    parser.parse(text);
} catch (err) {
    if (err.code == "INCONSISTENT_INPUT") {
        parser.parse(err.properText);
    } else {
        throw err;
    }
}
date = parser.getDate();

Instead of making Z token support :, I would suggest you to create a new token that represent timezone with :.

@MartinNuc

Copy link
Copy Markdown
Contributor Author

Indeed. I thought tokens are somewhat consistent with angular date filter but i've just noticed milliPrefix token. I agree to create a custom token for timezone with : sign. Thanks for your consultation. I will make PR during this week.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants