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

Fix mt_dateFromISOString #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hayashi311
Copy link

When [NSLocale currentLocale] is not en_* (like ja_JP) formatter can't parse ISO string.

@yas375
Copy link
Contributor

yas375 commented Mar 27, 2015

The fix looks good to me! I remember I had similar problems (I'm not using MTDate's ISO date formatter, but my own). Would be nice also to have a unit test which covers this. Let me know if you don't want or don't have a time to write a unit test for this and I'm happy to help with it :)

@hayashi311 hayashi311 force-pushed the fix/date-from-iso-string branch from 0066477 to 24236af Compare April 23, 2015 02:48
@hayashi311
Copy link
Author

@yas375 Thank you for your offer.
I'm trying to write a unit test. But it's difficult.

Settings for reproduce

The following setting occurs this bug. And this pull request resolves the problem.

  • Date & Time > 24-hour time > off
  • iPhone Language > Japanese
  • Region > Japan
  • Calendar > Japanese

When I turn 24-hour time on , It works.
This is hard to understand for me. Maybe NSDateFormatter is depends on iOS Date & Time setting.

The Problem is there is no "Date & Time" in the setting.app of iOS simulator.

@yas375
Copy link
Contributor

yas375 commented Apr 24, 2015

@hayashi311 thanks for trying to test this! I'll try to figure this out on the weekend.

@yas375
Copy link
Contributor

yas375 commented May 5, 2015

I'm sorry, I didn't get a chance to write a test for this yet.

@atomkirk I think if you could also verify the fix with manual steps, then it worth to merge in. There is similar report too #58

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