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

Normative: Require at least four digits in string representations of negative years #1407

Merged
merged 3 commits into from Apr 23, 2019

Conversation

Projects
None yet
5 participants
@gibson042
Copy link
Contributor

commented Jan 14, 2019

Fixes #1035

@ljharb ljharb requested review from zenparsing, bterlson, tc39/ecma262-editors and ljharb Jan 14, 2019

@littledan

This comment has been minimized.

Copy link
Member

commented Jan 28, 2019

Spec text looks good to me. FWIW, it seems like ChakraCore and SpiderMonkey currently ship these semantics.

Show resolved Hide resolved spec.html Outdated
Show resolved Hide resolved spec.html Outdated
Show resolved Hide resolved spec.html Outdated

@ljharb ljharb requested review from tc39/ecma262-editors and removed request for bterlson Feb 28, 2019

gibson042 added a commit to tc39/agendas that referenced this pull request Mar 16, 2019

gibson042 added a commit to gibson042/agendas that referenced this pull request Mar 16, 2019

@gibson042

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@gibson042

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

XS (and only XS) seems to always format negative years with six digits: https://circleci.com/gh/tc39/test262/340?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link#action-107

This is interesting because that behavior is required of Date.prototype.toISOString for conformance with ISO 8601, but toDateString/toUTCString/etc. have no such requirement. It superficially makes sense here, but gets trickier when we remember ISO also requires a plus sign for representing years after 9999. In other words, the same conformance with ISO 8601 that motivates use of six digits to represent every negative year from those methods would also motivate use of a plus sign and six digits to represent large positive years.

I personally foresee more problems than benefits stemming from e.g. "Mon, 01 Jul +065535 10:23:42 GMT", so I'm inclined to keep the tests as they are (requiring the minimum number of digits greater than or equal to 4, with no tainting from ISO 8601 year expansion).

@leobalter leobalter added has tests and removed needs tests labels Apr 2, 2019

leobalter added a commit to tc39/test262 that referenced this pull request Apr 2, 2019

Add tests for string representations of Date objects with negative ye…
…ars (#2114)

* Add tests for string representations of Date objects with negative years

Ref tc39/ecma262#1035
Ref tc39/ecma262#1407
@gibson042

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

Update: I have introduced a StringPad operation as suggested above by @ljharb, for precisely defining the representation of negative years in a way that prohibits e.g. "-001234" (as undesirably exhibited by XS).

@ljharb

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

@gibson042 5e0b005 seems editorial to me, and something i'd love to land separately/first. Would you be willing to make a separate PR for that commit? (you can leave it here, ofc, it will rebase out after the other one lands)

@ljharb ljharb force-pushed the gibson042:gh-1035-negative-year-date-strings branch from 87d27bf to c414add Apr 17, 2019

@ljharb

ljharb approved these changes Apr 17, 2019

@ljharb ljharb self-assigned this Apr 17, 2019

gibson042 added some commits Jan 14, 2019

@ljharb ljharb force-pushed the gibson042:gh-1035-negative-year-date-strings branch from c414add to f158df1 Apr 23, 2019

@ljharb ljharb merged commit f158df1 into tc39:master Apr 23, 2019

1 check was pending

continuous-integration/travis-ci/push The Travis CI build is in progress
Details

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 29, 2019

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 29, 2019

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.