[Webkit-unassigned] [Bug 44632] Date.parse not ES5 compliant, cannot parse standard Date Time String format

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44632


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70673|review?                     |review-
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2010-10-13 18:06:52 PST ---
(From update of attachment 70673)
View in context: https://bugs.webkit.org/attachment.cgi?id=70673&action=review

review- because of the decimal separator issue

> JavaScriptCore/wtf/DateMath.cpp:566
> +    static unsigned int daysPerMonth[] = {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};

We normally use “unsigned”, not “unsigned int”.

> JavaScriptCore/wtf/DateMath.cpp:569
> +    unsigned int month = 0, day = 0, hours = 0, minutes = 0;

We normally use “unsigned”, not “unsigned int”. We don’t declare multiple variables on a single line.

> JavaScriptCore/wtf/DateMath.cpp:574
> +    // NOTE: this is a bit more lenient on the year string than ES5 specifies:
> +    //       instead of restricting to 4 digits (or 6 digits with mandatory +/-),
> +    //       it accepts any integer value. Consider this an implementation fallback.

We use sentence format for comments, meaning the first word should be capitalized.

We don’t put NOTE in front of comments.

We don’t indent subsequent lines of comments in this fashion.

> JavaScriptCore/wtf/DateMath.cpp:576
> +    int numFieldsParsed = sscanf(dateString, "%d-%2u-%2u T %2u:%2u:%lf %6s",
> +                                 &year, &month, &day, &hours, &minutes, &seconds, timeZoneBuffer);

We don’t indent subsequent lines in this fashion.

Typically sscanf has poor performance. Would you be willing to write the parsing without using that function?

The sscanf function will determine which decimal point character based on the POSIX locale, and I presume we do not want that behavior here, so the seconds parsing is a bit of a problem.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.