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 for #1593 added some date tostring options #1596

Closed
wants to merge 2 commits into from

Conversation

voronoipotato
Copy link
Contributor

Needs some test cases and some testing. I haven't even built fable yet because I've not gotten that far yet but this is where I am right now.

@voronoipotato voronoipotato changed the title added some date tostring options Fix for #1593 added some date tostring options Oct 6, 2018
@voronoipotato
Copy link
Contributor Author

#1593

@voronoipotato
Copy link
Contributor Author

I guess feel free to close this PR

@alfonsogarciacaro
Copy link
Member

Thanks for your PR @voronoipotato! Maybe we don't need to close it in full, what's in the PR, only localization? I think it'll be difficult to support Globalization as in .NET because it's quite a big amount of work and that may increase the bundle sizes. For the same reasons, I'd prefer to avoid external dependencies in fable-core (like moment-js), but we can discuss about that.

@voronoipotato voronoipotato reopened this Oct 8, 2018
@voronoipotato
Copy link
Contributor Author

I'll reopen for now to see if anything may be useful. I wouldn't say that it was "for" globalization but rather some of the formatting does not take into consideration other cultures. The goal I had in doing this PR was to make copy pasting F# into fable just a little bit easier but I did have concerns about it only being useful for westerners potentially mostly for Americans (I know very little about other cultures).

@voronoipotato
Copy link
Contributor Author

I added tostring options with the intention of improving compatibility with Core. I added (f: milliseconds) , (t: AM/PM), dddd Weekdays (english), ddd Weekdays Short (english), mmmm Months (english), mmm Months Short (english), in addition I added several single letter tostring options. d,D,f,F,g,G,m,M,t,T,U . The single letter ToString format for dates exists as a convenience and allows the user to quickly format a date in a useful way. Date.ToString("d") for example is equivalent to Date.ToString("M/d/yyyy") however you can see how this may be less useful for someone who doesn't use middle endian dates. There definitely are bits that would require no localization, and many of these are the default if I'm not mistaken in .Net, which wouldn't surprise me given that the Microsoft is an American company. However I am an American, and have never programmed anything in .Net that needed localization so I'm not exactly any authority on the subject.

@ncave
Copy link
Collaborator

ncave commented Oct 9, 2018

Perhaps later we can move all the date stuff into Date.ts, since Util.ts is getting quite big.
A few tests to cover the new options will go a long way. "A test a day keeps the doctor away."

@voronoipotato
Copy link
Contributor Author

That sounds wise ncave. I'll give it a shot.

@alfonsogarciacaro
Copy link
Member

Hi @voronoipotato! What's the status of this? Will you include some tests to see what the PR is trying to solve? Thank you!

@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 3 times, most recently from dd1aeb3 to 027307b Compare February 21, 2019 22:20
@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 2 times, most recently from 13a9f4e to dbf433a Compare March 13, 2019 14:49
@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 2 times, most recently from 8c88154 to e2c26ad Compare April 1, 2019 22:19
@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 2 times, most recently from 9f9d44d to 6ae4cba Compare May 10, 2019 14:10
@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 3 times, most recently from 3a9f995 to e549bb1 Compare September 4, 2019 23:35
@alfonsogarciacaro alfonsogarciacaro force-pushed the master branch 8 times, most recently from 7fe56ba to e549bb1 Compare September 10, 2019 08:36
@alfonsogarciacaro alfonsogarciacaro changed the base branch from master to nagareyama December 3, 2020 06:23
@voronoipotato
Copy link
Contributor Author

This isn't needed anymore is it? I am dumb as bricks and missed the ping for a few years now.

@voronoipotato voronoipotato changed the base branch from nagareyama to main December 9, 2022 04:41
@MangelMaxime
Copy link
Member

This PR being quite old I think we can close it for now.

There is #3692 which tracks the task to improve supported format for DateTime. If someone want to revive it then can either adapt the original mscord code or take inspiration from this PR too.

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.

4 participants