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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 16 09:51:39 PDT 2009


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





--- Comment #31 from Timothy Hatcher <timothy at hatcher.name>  2009-09-16 09:51:39 PDT ---
(In reply to comment #28)
> - 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.

This is fine, but we don't use all caps. Should be CamelCase.


> - 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. 

This is a good solution.


> 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).  

It could be passed to the ObjectPropertiesSection constructor to store and
hold. There is a argument on the constructor already for the tree element
constructor.

> 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.

The solution is good. Better than it was before. But I don't see what is wrong
with the class hierarchies.

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



More information about the webkit-unassigned mailing list