Implement Date.prototype.toISOString() #418
Conversation
egavrin
added
ecma builtins
|
Will this change unify smth later? |
| @@ -32,6 +32,10 @@ assert (new Date (NaN).toDateString () == "Invalid Date"); | |||
| assert (new Date ("2015-02-13").toDateString () == "2015-02-13"); | |||
| assert (new Date ("2015-07-08T11:29:05.023").toDateString () == "2015-07-08"); | |||
|
|
|||
| assert (new Date (NaN).toISOString () == "Invalid Date"); | |||
| assert (new Date ("2015-07-16").toISOString () == "2015-07-16T00:00:00.000Z"); | |||
| assert (new Date ("2015-07-16T11:29:05.023").toISOString () == "2015-07-16T11:29:05.023Z"); | |||
There was a problem hiding this comment.
Please move the new test cases to the end of the file and add test for TypeError too.
rtakacs
force-pushed
the
date_toisostring
branch
from
0102809
to
a500c3b
Compare
| * | ||
| * See also: | ||
| * ECMA-262 v5, 15.9.5.2 | ||
| * ECMA-262 v5, 15.9.5.43 |
There was a problem hiding this comment.
Could we keep the order of the functions as the ECMA standard declares them?
There was a problem hiding this comment.
All builtin function names are already declared in ecma-builtin-date-prototype.inc.h. So we don't need to change the position of these functions.
There was a problem hiding this comment.
Yes, we could. Added forward declaration for *to_iso_string.
There was a problem hiding this comment.
Ahh sorry, the declaration is already in the header.
rtakacs
force-pushed
the
date_toisostring
branch
2 times, most recently
from
577fc6d
to
f1d10fd
Compare
|
Looks good to me. |
|
+1 lgtm |
| { | ||
| ecma_number_t milliseconds = ecma_date_ms_from_time (*prim_value_num_p); | ||
| ecma_string_t *output_str_p = ecma_get_magic_string (LIT_MAGIC_STRING__EMPTY); | ||
| ecma_date_insert_num_with_sep (&output_str_p, milliseconds, LIT_MAGIC_STRING_UTC_TIME_ZONE_U, 3); |
There was a problem hiding this comment.
15.9.5.2 Date.prototype.toString ( )
This function returns a String value. The contents of the String are implementation-dependent, but are intended to represent the Date in the current time zone in a convenient, human-readable form.
15.9.5.43 Date.prototype.toISOString ( )
This function returns a String value represent the instance in time represented by this Date object. The format of the String is the Date Time string format defined in 15.9.1.15. All fields are present in the String. The time zone is always UTC, denoted by the suffix Z. If the time value of this object is not a finite Number a RangeError exception is thrown.
As output of the toString and the toISOString are different only in timezone part (toISOString outputs in UTC timezone and toString should output in "current" timezone), maybe we should extract the code to a helper with arguments is_current_timezone, and add TODO comment about implementing output in "current" timezone mode.
There was a problem hiding this comment.
So, the helper could be called from both toString and toISOString implementations.
There was a problem hiding this comment.
You are right, I missed that the time zone is different from toString and toISOString. The helper is a good solution. The toUTCString could use it as well.
rtakacs
force-pushed
the
date_toisostring
branch
from
f1d10fd
to
1aa4c0d
Compare
|
Looks good to me |
JerryScript-DCO-1.0-Signed-off-by: Roland Takacs rtakacs.u-szeged@partner.samsung.com
rtakacs
force-pushed
the
date_toisostring
branch
from
1aa4c0d
to
8e7d168
Compare
|
LGTM |
|
Merged 63083b3 |
Moved toString() logic into the toISOString() method extended with a Z postfix as the standard said.
Since the toString() is "implementation-dependent" the toString() calls the toISOString() method.