We should always be sorting strings as `x1 < x2 < x10` instead of `x1 < x10 < x2`.
Created attachment 314166 [details] Patch
Comment on attachment 314166 [details] Patch Utility functions are testable. Other than lack of tests, looks good.
Comment on attachment 314166 [details] 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
Created attachment 314206 [details] Patch
Comment on attachment 314206 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314206&action=review r=me > LayoutTests/inspector/unit-tests/string-utilities.html:44 > + InspectorTest.expectEqual("1".extendedLocaleCompare("2"), -1, `"1" < "2"`); > + InspectorTest.expectEqual("2".extendedLocaleCompare("1"), 1, `"2" > "1"`); > + InspectorTest.expectEqual("2".extendedLocaleCompare("10"), -1, `"2" < "10"`); > + InspectorTest.expectEqual("10".extendedLocaleCompare("2"), 1, `"10" > "2"`); > + InspectorTest.expectEqual("1".extendedLocaleCompare("10"), -1, `"1" < "10"`); > + InspectorTest.expectEqual("10".extendedLocaleCompare("1"), 1, `"10" > "1"`); How does: "a2" compare to "a10"? Can we have a test? > Source/WebInspectorUI/UserInterface/Base/Utilities.js:676 > + value: function(other) Style: value(other)
Created attachment 314287 [details] Patch
Comment on attachment 314287 [details] Patch Clearing flags on attachment: 314287 Committed r219014: <http://trac.webkit.org/changeset/219014>
All reviewed patches have been landed. Closing bug.