Implement Date.prototype.toDateString and Date.prototype.toTimeString #360
Conversation
LaszloLango
added
ecma builtins
|
@ruben-ayrapetyan, sorry if I merged the #336 too early. I did everything you asked. If I missed something or there are any other issues with it, then we can fix them in this PR too, because is related to it. |
| ECMA_BUILTIN_CP_UNIMPLEMENTED (this_arg); | ||
| ecma_completion_value_t ret_value = ecma_make_empty_completion_value (); | ||
|
|
||
| if (ecma_object_get_class_name (ecma_get_object_from_value (this_arg)) != LIT_MAGIC_STRING_DATE_UL) |
There was a problem hiding this comment.
What would happen if I call the function this way?:
Date.prototype.toDateString.call(-1)
LaszloLango
force-pushed
the
date_todatestring_and_totimestring
branch
2 times, most recently
from
90f1a78 to
7ca0470
Compare
|
@ruben-ayrapetyan, I've updated the PR based on comments on #336. I've added the fixes as a separate commit as you asked. Please check. |
| } | ||
| else if (ecma_object_get_class_name (ecma_get_object_from_value (this_arg)) != LIT_MAGIC_STRING_DATE_UL) | ||
| { | ||
| ret_value = ecma_raise_type_error ("Incompatible type"); |
There was a problem hiding this comment.
As actions under the conditions of the if and else if blocks are the same, and conditions are not complex, it is better to merge the blocks.
|
After fixing, #360 (comment) and likewise, the changes would look good to me. |
Thank you for the update. |
JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com
JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com
LaszloLango
force-pushed
the
date_todatestring_and_totimestring
branch
from
7ca0470 to
e1b9fb6
Compare
|
@ruben-ayrapetyan, fixed. Sorry again for inconvenience with #336. @galpeter, please check. |
|
lgtm |
| for (uint32_t i = length - 1; i > 0 && num < pow (10,i); i--) | ||
| uint32_t first_index = length - 1u; | ||
| ecma_number_t power_i = (ecma_number_t) pow (10, first_index); | ||
| for (uint32_t i = first_index; i > 0 && num < power_i; i--, power_i /= 10) |
There was a problem hiding this comment.
What is the range of length? I mean what happens if power_i cannot precisely represent the value? Is that possible?
There was a problem hiding this comment.
No it is not possible, the range is 1-4.
| assert (new Date(NaN) == "Invalid Date"); | ||
| assert (new Date("2015-02-13") == "2015-02-13T00:00:00.000"); | ||
| assert (new Date("2015-07-08T11:29:05.023") == "2015-07-08T11:29:05.023"); | ||
| assert (new Date (NaN) == "Invalid Date"); |
There was a problem hiding this comment.
why don't we use 3 equal signs?
There was a problem hiding this comment.
I always write tests like this. :) Should I change this? Why would be 3 equal signs better?
There was a problem hiding this comment.
Because it also checks the type, so it is more strict than the two. I would recommend to use it in the future.
Later you can make a style patch to change these.
There was a problem hiding this comment.
Ok, thanks for explain. I'll raise an issue for this.
|
LGTM |
|
@ruben-ayrapetyan, still good? May I push this? |
|
@LaszloLango, yes, the changes look good to me. |
JerryScript-DCO-1.0-Signed-off-by: László Langó llango.u-szeged@partner.samsung.com