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

Add support for %k, %l, %u, %W, missing test for %e and extra test for %Z #14

Merged
merged 1 commit into from
Aug 24, 2015

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Aug 21, 2015

I'm depending on a few more strftime-format string specifiers - %k, %l are hours without leading zero, %u is day of week with sunday=7 and %W is the week number, this PR introduces them.

Technically, they're not strictly compatible because that would require a leading space for %k & %l but it's the closest we can get without a full rewrite.

Also, the entries for z & Z were out order alphabetically.

This also adds a missing test for %e, an additional date - needed for the leading-zero/nothing difference, and a test that tests that %Z is still passed to moment - according to moment#162, it should keep working when moment-timezone is also used.

@himdel
Copy link
Contributor Author

himdel commented Aug 21, 2015

(just noticed the extra whitespace change in the spec, I can fix that if you mind..)

@himdel
Copy link
Contributor Author

himdel commented Aug 21, 2015

For %Z, the other option could be to translate it as [UTC]ZZ instead of z - getting a date with, say.. "UTC-04:00" is better than getting no timezone at all.

@benjaminoakes
Copy link
Owner

This is great. Thank you for your contribution!

benjaminoakes added a commit that referenced this pull request Aug 24, 2015
Add support for %k, %l, %u, %W, missing test for %e and extra test for %Z
@benjaminoakes benjaminoakes merged commit 99e2515 into benjaminoakes:master Aug 24, 2015
@benjaminoakes
Copy link
Owner

I've released this as v0.1.4 on npm and bower.

@himdel
Copy link
Contributor Author

himdel commented Aug 25, 2015

@benjaminoakes No problem, thanks :)

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