[webkit-reviews] review denied: [Bug 173984] Web Inspector: Default string comparisons to treat numeric characters as numbers : [Attachment 314166] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 173984: Web Inspector: Default string comparisons to treat numeric
characters as numbers
https://bugs.webkit.org/show_bug.cgi?id=173984

Attachment 314166: Patch

https://bugs.webkit.org/attachment.cgi?id=314166&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 314166
  --> https://bugs.webkit.org/attachment.cgi?id=314166
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314166&action=review

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:1492
> +function numericStringComparator(a, b)
> +{
> +    return a.localeCompare(b, undefined, {numeric: true});
> +}

I totally agree with Matt that we should have a quick unit-test for this. See:
LayoutTests/inspector/unit-tests/*

Did this just replace every instance of localeCompare? I think we should name
this along the lines of localeCompare otherwise I'm going to forget about it
and end up adding localeCompare.

How about:

    numberAwareLocaleCompare() // Gives an idea what it does
    extendedLocaleCompare() // Indicates its somehow better thane just
localeCompare
    String.prototype.extendedLocaleCompare // like
Number.prototype.bytesToString