[Webkit-unassigned] [Bug 27329] Inspector: Properties Should be Sorted more Naturally

[bugzilla-daemon at webkit.org]

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


Oliver Hunt <oliver at apple.com> changed:

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




--- Comment #7 from Oliver Hunt <oliver at apple.com>  2009-07-16 01:26:47 PDT ---
(From update of attachment 32829)
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 45964)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,13 @@
> +2009-07-15  Joseph Pecoraro  <joepeck02 at gmail.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Inspector: Properties Should be Sorted more Naturally
> +        https://bugs.webkit.org/show_bug.cgi?id=27329
> +
> +        * inspector/front-end/utilities.js:
> +        (Object.sortedProperties): added alphaNumSort sorting function
> +
>  2009-07-15  Adam Langley  <agl at google.com>
>  
>          No review: reverting previous change.
> Index: WebCore/inspector/front-end/utilities.js
> ===================================================================
> --- WebCore/inspector/front-end/utilities.js	(revision 45963)
> +++ WebCore/inspector/front-end/utilities.js	(working copy)
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2007 Apple Inc.  All rights reserved.
> + * Copyright (C) 2007, 2009 Apple Inc.  All rights reserved.
>   *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions
> @@ -96,10 +96,43 @@ Object.describe = function(obj, abbrevia
>  
>  Object.sortedProperties = function(obj)
>  {
> +
> +    function alphaNumSort(a,b) {
> +        a = a.toString();
> +        b = b.toString();
In our usage alphaNumSort is always passed strings so these toString calls are
unnecessary, and are definitely not free, so you shouldn't have them

> +        if (a===b)
should have spaces around ===

> +            return 0;
> +
> +        var diff = 0;
> +        var chunk = /^\d+|^\D+/;
> +        var chunka, chunkb, anum, bnum;
> +        while (diff === 0) {
> +            if (!a && b)
> +                return -1;
> +            if (!b && a)
> +                return 1;
> +            chunka = a.match(chunk)[0];
> +            chunkb = b.match(chunk)[0];
> +            anum = +chunka+1; // quick falsey check if its a number
> +            bnum = +chunkb+1; // quick falsey check if its a number
This seems unnecessarily complex -- why "+1" ?  aside from that the second +
should have spaces on either side.

> +            if (anum && !bnum)
> +                return -1;
> +            if (bnum && !anum)
> +                return 1;
> +            if (anum && bnum)
> +                diff = chunka - chunkb;
> +            else if (chunka !== chunkb)
> +                return (chunka<chunkb) ? -1 : 1;
Should have spaces around <

> +            a = a.substring(chunka.length);
> +            b = b.substring(chunkb.length);
> +        }
> +        return diff;
> +    }
> +    

Otherwise this basically seems fine, just needs my comments addressed

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