Normative: Specify Date string conversion functions #848

littledan · 2017-03-17T16:51:43Z

Implementations seem to agree on semantics here for the most
part, modulo timezone strings, which are left implementation-defined.

Addresses part of #845

claudepache · 2017-03-18T00:19:39Z

spec.html

        <!-- es6num="20.3.4.41.1" -->
        <emu-clause id="sec-todatestring" aoid="ToDateString">
          <h1>Runtime Semantics: ToDateString( _tv_ )</h1>
          <p>The following steps are performed:</p>
          <emu-alg>
            1. Assert: Type(_tv_) is Number.
            1. If _tv_ is *NaN*, return `"Invalid Date"`.
-            1. Return an implementation-dependent String value that represents _tv_ as a date and time in the current time zone using a convenient, human-readable form.
+            1. Let _t_ be LocalTime(_tv_).
+            1. Return the String value formed by concatenating DateString(_t_), `" "`, and TimeString(_t_), TimeZoneString(_tv_).


... `" "`, TimeString(_t_), and TimeZoneString(_tv_). (the word and is misplaced)

Fixed, thanks

littledan · 2017-03-18T00:25:05Z

@domenic Do you have good suggestions for how to write tests here? I'm not sure how to do it in a way that will work in all time zones. (test262 has repeatedly had issues where tests were broken in certain timezones.)

claudepache · 2017-03-18T00:27:08Z

spec.html

+          <emu-alg>
+            1. Assert: Type(_tv_) is Number.
+            1. Assert: _tv_ is not *NaN*.
+            1. Return String value formed by concatenating _result_, ToString(HourFromTime(_tv_)), `":"`, ToString(MinFromTime(_tv_)), `":"`, ToString(SecFromTime(_tv_)) and `" GMT"`


delete extraneous result variable

claudepache · 2017-03-18T01:09:49Z

spec.html

+            1. Let _offset_ be (LocalTZA + DaylightSavingTA(_tv_)) / msPerSecond.
+            1. Assert: _offset_ is an integer, and -100000 &lt; _offset_ &lt; 100000.
+            1. If _offset_ &ge; 0, let _offsetSign_ be `"+"`; otherwise, let _offsetSign_ be `"-"`.
+            1. Let _offsetString_ be abs(_offset_) formatted as a four-digit number, padded to the left with zeros if necessary.


The offset needs to be formatted as HHMM, not as SSSS. The four preceding steps (starting with Let _offset_) should be replaced with:

Let offset be LocalTZA + DaylightSavingTA(tv) /* (NB: value in milliseconds) */

If offset ≥ 0, let offsetSign be "+"; otherwise, let offsetSign be "-".

Let offsetMin be the String representation of MinFromTime(abs(offset)), formatted as a two-digit number, padded to the left with a zero if necessary.

Let offsetHour be the String representation of HourFromTime(abs(offset)), formatted as a two-digit number, padded to the left with a zero if necessary.

And the last step of the algorithm, below:

Let offsetString be the concatenation of offsetSign, offsetHour, offsetMin, and tzName.

Thanks, of course! Uploaded a new version to use your text.

domenic · 2017-03-18T05:17:41Z

Do you have good suggestions for how to write tests here? I'm not sure how to do it in a way that will work in all time zones. (test262 has repeatedly had issues where tests were broken in certain timezones.)

Well, you can certainly test the Invalid Date cases.

You should be able to run regexps to make sure the patterns are met for all methods, including the time zone format restriction.

You should be able to test .toUTCString() exactly on its entire range of inputs, including extreme ones like 0, +/- Infinity, Number.(MIN/MAX)_SAFE_INTEGER, and Number.(MIN/MAX)_VALUE.

You should be able to pick times in the middle of a given month so that timezone changes can't change the month string, thus allowing you to test the month name mapping even for toString().

You might be able to pick times such that in every timezone they fall on the same day of the week, thus allowing testing the week day mapping for toString(), but I am not sure.

ljharb · 2017-03-18T05:29:40Z

spec.html

+          1. Let _O_ be this Date object.
+          1. Let _tv_ be ? thisTimeValue(_O_).
+          1. If _tv_ is *NaN*, return `"Invalid Date"`.
+          1. Return the String value formed by concatenating DateString(_tv_), `" "`, and TimeString(_tv_).


ljharb · 2017-03-18T05:29:52Z

spec.html

        <!-- es6num="20.3.4.41.1" -->
        <emu-clause id="sec-todatestring" aoid="ToDateString">
          <h1>Runtime Semantics: ToDateString( _tv_ )</h1>
          <p>The following steps are performed:</p>
          <emu-alg>
            1. Assert: Type(_tv_) is Number.
            1. If _tv_ is *NaN*, return `"Invalid Date"`.
-            1. Return an implementation-dependent String value that represents _tv_ as a date and time in the current time zone using a convenient, human-readable form.
+            1. Let _t_ be LocalTime(_tv_).
+            1. Return the String value formed by concatenating DateString(_t_), `" "`, TimeString(_t_), and TimeZoneString(_tv_).


for the space, the convention i've seen (and followed in padStart/padEnd) to avoid whitespace ambiguity is " " (a String consisting of 0x0020 SPACE)

@bterlson, thoughts on this?

@ljharb I was going to clean this up myself (working on it soon). Planning on replacing all spaces with "the code unit 0x0020 (SPACE)"

ljharb · 2017-03-18T05:30:22Z

spec.html

+            1. Assert: _offset_ is an integer, and -100000 &lt; _offset_ &lt; 100000.
+            1. If _offset_ &ge; 0, let _offsetSign_ be `"+"`; otherwise, let _offsetSign_ be `"-"`.
+            1. Let _offsetString_ be abs(_offset_) formatted as a four-digit number, padded to the left with zeros if necessary.
+            1. Let _tzName_ be an implementation-defined string, either `""` or a string of the form `" ("` _name_ `")"` where _name_ is an implementation-defined timezone name.


similarly (and throughout this PR where strings appear), this might want to specify the code units for the space and parens, to avoid ambiguity.

What ambiguity would there be? There are many other places in the spec that use characters with multiple Unicode code points that render similarly or the same (e.g., every a character).

¯\_(ツ)_/¯ there are many characters for whitespace. Being precise about the code point ensures no confusion, even with a non-english-centric implementation.

claudepache · 2017-03-18T06:17:38Z

You might be able to pick times such that in every timezone they fall on the same day of the week, thus allowing testing the week day mapping for toString(), but I am not sure.

You cannot. UTC+14 will always be 1 or 2 days ahead of UTC-12.

Implementations seem to agree on semantics here for the most part, modulo timezone strings, which are left implementation-defined. Addresses part of tc39#845

Also fix up the text of DateString().

This actually gets into an area of apparent disagreement between implementations. For years less than 1000, JSC and SpiderMonkey implement the specification here, whereas V8 pads to four digits with spaces, and ChakraCore does not pad the number.

Following the proposed specification in tc39/ecma262#848

littledan · 2017-03-23T14:07:03Z

There's a small part of this which actually differs between browsers: When printing the year, if the number is below 1000, JSC and SpiderMonkey pad 0s to the left, V8 pads with spaces, and ChakraCore doesn't pad. This patch goes with the SpiderMonkey/JSC behavior. Before merging it, we should make sure that those engines are OK with the potential compatibility impact of this change.

@ajklein @bterlson What do you think?

claudepache · 2017-03-23T15:35:18Z

There's a small part of this which actually differs between browsers: When printing the year, if the number is below 1000, JSC and SpiderMonkey pad 0s to the left, V8 pads with spaces, and ChakraCore doesn't pad.

We should definitely pad with 0s, so that years between 0 and 99 look unambiguous, and we will be able to correct the following glitch of Date.parse() (tested on Firefox, Safari, Chrome and Edge):

var d = new Date;
d.setFullYear(97);
d.toString(); // variation on: Sat Mar 23 0097 16:04:11 GMT+0100 (CET)
var d2 = new Date(Date.parse(d.toString()));
d2.toString(); // variation on: Sun Mar 23 1997 16:04:11 GMT+0100 (CET) //oops, wrong year

jungshik · 2017-05-04T21:01:57Z

spec.html

+          <emu-alg>
+            1. Assert: Type(_tv_) is Number.
+            1. Assert: _tv_ is not *NaN*.
+            1. Let _offset_ be (LocalTZA + DaylightSavingTA(_tv_)) / msPerSecond.


This will be in conflict with #778 that drops DaylightSavingTA(tv) and change LocalTZA to take two params. We have to sort that out somehow.

You're right; this is written against the current spec, but needs to be updated against the other one (based on whichever lands second). I guess rather than LocalTZA, we'd use LocalTimeZoneAdjustment(tz, true), right?

Ok. this one was landed first. Then, the other one (#778) has to revise this part as well.

jungshik · 2017-05-04T21:04:39Z

spec.html

+            1. Assert: _offset_ is an integer, and -100000 &lt; _offset_ &lt; 100000.
+            1. If _offset_ &ge; 0, let _offsetSign_ be `"+"`; otherwise, let _offsetSign_ be `"-"`.
+            1. Let _offsetString_ be abs(_offset_) formatted as a four-digit number, padded to the left with zeros if necessary.
+            1. Let _tzName_ be an implementation-defined string, either `""` or a string of the form `" ("` _name_ `")"` where _name_ is an implementation-defined timezone name.


Would it make sense to give a recommendation while leaving it still up to an implementation about tzName?
If it does, it can be recommended to use the canonicalized timezone in Ecma402. Perhaps, cross-referencing is not a good idea?
Just my 2 cents.

I meant for this first patch to be purely 100% descriptive. Maybe a change like this could be in a follow-on patch?

As for making that recommendation: I don't know whether we can make it in good faith in the spec yet, as we don't know whether it's web-compatible yet. Maybe the right thing to do would be to try it out in one browser and see if it works. A good thing to know from the committee beforehand would be--would everyone be up for specifying this behavior, assuming it's web-compatible?

jungshik

Thank you for the PR ! A couple of comments.

rwaldron · 2017-05-25T16:12:11Z

Sorry for the label noise.

littledan · 2017-05-29T14:41:06Z

I previously removed the needs tests label because tests are at tc39/test262#930 (though they need a small fix).

rwaldron · 2017-06-22T20:23:22Z

@littledan I did a follow up from @leobalter's initial review and merged the test262 PR

domenic added the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Mar 17, 2017

claudepache reviewed Mar 18, 2017

View reviewed changes

claudepache suggested changes Mar 18, 2017

View reviewed changes

ljharb reviewed Mar 18, 2017

View reviewed changes

littledan added 5 commits March 22, 2017 20:41

Normative: Specify Date string conversion functions

94ab5c3

Implementations seem to agree on semantics here for the most part, modulo timezone strings, which are left implementation-defined. Addresses part of tc39#845

Move 'and'

8dff62f

Delete extraneous '_result_'

702d612

Fix GMT+-HHMM with @claudepache's text

7039152

toUTCString has a comma after the day of the week

873e0d2

Also fix up the text of DateString().

littledan force-pushed the date-tostring branch from 21923ce to 873e0d2 Compare March 22, 2017 20:03

littledan added 2 commits March 23, 2017 00:24

toUTCString swaps month and day

59dfec6

littledan added a commit to littledan/test262 that referenced this pull request Mar 23, 2017

Tests for "Invalid Date" in more cases

bd685e5

Following the proposed specification in tc39/ecma262#848

littledan mentioned this pull request Mar 23, 2017

Date toString tests tc39/test262#930

Merged

jungshik reviewed May 4, 2017

View reviewed changes

rwaldron added has consensus This has committee consensus. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 and removed has consensus This has committee consensus. labels May 25, 2017

littledan removed the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label May 29, 2017

bterlson merged commit 5d4acf3 into tc39:master May 30, 2017

jungshik mentioned this pull request Jun 8, 2017

Make LocalTZA take 't' and 'isUTC' and drop DSTA(t). #778

Merged

rwaldron added the has test262 tests label Jun 22, 2017

ljharb mentioned this pull request Jan 25, 2019

Where's the proposal for the Date.prototype.toString change in ES2018? tc39/proposals#191

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Specify Date string conversion functions #848

Normative: Specify Date string conversion functions #848

littledan commented Mar 17, 2017

claudepache Mar 18, 2017

littledan Mar 18, 2017

littledan commented Mar 18, 2017

claudepache Mar 18, 2017

littledan Mar 18, 2017

claudepache Mar 18, 2017

littledan Mar 18, 2017

domenic commented Mar 18, 2017

ljharb Mar 18, 2017

ljharb Mar 18, 2017

ljharb May 30, 2017

bterlson May 30, 2017

ljharb Mar 18, 2017

littledan Mar 18, 2017

ljharb Mar 19, 2017 •

edited

Loading

claudepache commented Mar 18, 2017

littledan commented Mar 23, 2017 •

edited

Loading

claudepache commented Mar 23, 2017 •

edited

Loading

jungshik May 4, 2017

littledan May 4, 2017

jungshik Jun 8, 2017

jungshik May 4, 2017

littledan May 4, 2017

jungshik left a comment

rwaldron commented May 25, 2017

littledan commented May 29, 2017

rwaldron commented Jun 22, 2017

Normative: Specify Date string conversion functions #848

Normative: Specify Date string conversion functions #848

Conversation

littledan commented Mar 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented Mar 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Mar 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb Mar 19, 2017 • edited Loading

Choose a reason for hiding this comment

claudepache commented Mar 18, 2017

littledan commented Mar 23, 2017 • edited Loading

claudepache commented Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungshik left a comment

Choose a reason for hiding this comment

rwaldron commented May 25, 2017

littledan commented May 29, 2017

rwaldron commented Jun 22, 2017

ljharb Mar 19, 2017 •

edited

Loading

littledan commented Mar 23, 2017 •

edited

Loading

claudepache commented Mar 23, 2017 •

edited

Loading