[Webkit-unassigned] [Bug 27514] add support for watched expressions

[bugzilla-daemon at webkit.org]

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


Patrick Mueller <pmuellr at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #39648|                            |review?
               Flag|                            |
  Attachment #39610|0                           |1
        is obsolete|                            |




--- Comment #28 from Patrick Mueller <pmuellr at yahoo.com>  2009-09-16 09:13:58 PDT ---
Created an attachment (id=39648)
 --> (https://bugs.webkit.org/attachment.cgi?id=39648)
proposed patch - 2009/09/16

Since the previous patch:

- element added when user selects "Add" now uses a 'constant' for the name,
WebInspector.WatchExpressionsSection.NEW_WATCH_EXPRESSION, rather than just " "
as a special case, to make things a little clearer.  The value has also been
changed to "\xA0", which works as well as space where it needs to (evaluates
without issues, is stripped with trimWhitespace(), etc), and is less likely in
negatively interact with functioning in the future if for some reason someone
wants to use " " for a valid or special purpose in the expressions.

- WatchExpressionsSection.update() was previously calling
ObjectPropertiesSection._update().  I renamed the _update() method to
updateProperties() in all callers and the single implementation, to make it
public, and make the name a little more obvious.

- property compare has been jiggled around.  It's now available as a 'class
property' - WebInspector.ObjectPropertiesSection.PropertyComparer for
ObjectProperties* stuff, and
WebInspector.WatchExpressionsSection.PropertyComparer for Watch Expressions. 
The reason it can't easily be just a regular instance method, is for two
reasons:

   - it's not an instance method, it's a function - it doesn't get called with
a this.  It would be confusing to have it as an instance method (function on a
prototype), since there is no reason to ever call it that way.

   - it's used in two places in ObjectProperties*; once in
ObjectPropertiesSection to sort the top level list of properties, and then in
ObjectPropertyTreeElement to sort all the further nested properties.  That
second one needs to reference the comparer function somehow.

It's very confusing to sort out how the second reference should be obtained,
programmatically, as the hierarchies get tangly, and most confusingly, the
properties are sorted before being handed off to the element tree constructors
(which is one place you might think to hang this function).  

To make things a little more straight-forward, what I've done is added the
comparator functions to the relevent section contructors as properties (not on
the prototype, just properties off the constructor), as mentioned above.  I've
then added a further customization parameter in the top-level element building
function in ObjectPropertiesSection, now called updateProperties (see above),
to take an optional comparator.  This will default to use the version in
ObjectPropertiesSection.  WatchExpressions will pass in an override.  In the
second use of the comparator, in the ObjectPropertyTreeElement, I've hardcoded
the reference to the ObjectPropertiesSection, since today that's all anyone
uses.

I think the result is fairly easy to understand.  It doesn't neccessarily feel
like the best shape, but I think that's largely because the ObjectProperties*
class hierarchies are getting a bit convoluted in general.  It was the most
natural thing I came up with.  If we need to extend them further, it might be
time for some more serious refactoring.

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