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

Define locale options for :datetime, :date & :time #911

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

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Oct 19, 2024

Adds a new "basket" of date/time locale options that override defaults set in the current locale or an implementation-defined date/time operand value. These are optional, and can be made available on each of :datetime, :date & :time.

The hourCycle option is dropped from the :datetime field options (where it never belonged), and its functionality is replaced by the hour12=true|falsedate/time locale option.

This PR explicitly does not address the concerns raised in #866; that is a separate discussion.

Copy link
Member

@aphillips aphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like some of the thought behind this, but want to think about it more.

- `true`
- `false`
- `timeZone`
- valid identifier per [BCP175](https://www.rfc-editor.org/rfc/rfc6557)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't link the RFC. Link the BCP (e.g. https://www.rfc-editor.org/bcp/bcp175)... but... time zone identifiers have a ton of quirks in them. Also, we almost certainly want to allow offset time zones (e.g. GMT-01:23) and we may want to allow special sauce like metazones. CLDR has a bunch of stuff about this, but I'm too busy this morning to look up the precise reference. It's somewhere near here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to go with whatever; this reference was not changed from the earlier one we already included.

Copy link
Member

@aphillips aphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a refactor to "optional" is better than having them separate.

@@ -826,3 +812,24 @@ For more information, see [Working with Timezones](https://w3c.github.io/timezon
> The form of these serializations is known and is a de facto standard.
> Support for these extensions is expected to be required in the post-tech preview.
> See: https://datatracker.ietf.org/doc/draft-ietf-sedate-datetime-extended/

### Date and Time Locale Options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having thought about it, I think this is the wrong approach. The MAY means that these are optional options. We should just put them into the list for each function as optional

Doing so would solve the other problem I have, which is that timeZone is most definitely not part of locale (even if the -u extension provides a mechanism for transmitting the zone).

Comment on lines +830 to +831
- `timeZone`
- valid identifier per [BCP175](https://www.rfc-editor.org/rfc/rfc6557)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to do a bit more here. The ability to float and unfloat time values is actually rather important, even if developers don't always understand it. See: Working with Time and Timezones

Suggested change
- `timeZone`
- valid identifier per [BCP175](https://www.rfc-editor.org/rfc/rfc6557)
- `timeZone` **optional** (no default)
- valid identifier per [BCP175](https://www.rfc-editor.org/bcp/bcp175)
- `none` (removes the time zone, if present, to create a local/floating time)

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

Successfully merging this pull request may close these issues.

3 participants