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

[bugzilla-daemon at webkit.org]

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





--- Comment #27 from Patrick Mueller <pmuellr at yahoo.com>  2009-09-15 18:41:07 PDT ---
(In reply to comment #26)
> (From update of attachment 39610 [details])
> > +        properties.sort(rootTreeElementConstructor.prototype.comparePropertyNames);
> 
> This should just be rootTreeElementConstructor.comparePropertyNames, to allow
> instances to override the prototype.

Whats uber-confusing about this, is that the sorting takes place before passing
the names to the class which is going to handle them.  And it needs to be
inheritable, so that ScopeVariableTreeElement picks up the default from
ObjectProperties, but allowing the root collection of WatchExpressions to do
something different.  Took me a while to even get it to work correctly in all
cases with this scheme.

Let me noodle on this a bit more, clearly the code is more complicated than it
should be.  It might well be that making it instance-based customizable is
actually the simplest thing to do.

> > +            if (property.name === " ") 
> 
> Maybe everywhere you check for " " you should just trim whitspace
> (String.trimWhitespace in utilities.js) and look for "" or !property.name?
> 
> I would r+ this, but I would like to see the prototype thing fixed, even though
> it isn't an issue today.

I'm down to using a single " " character as the signal for "this is a newly
added element which needs to be edited".  Empty space, or actually, as you
suggest testing for (!whatever) is used everywhere else to signal "this item
needs to be deleted".  A little confusing.  Since I don't really use the " "
character productively (or don't need to), I'm thinking maybe I should making
this some kind of non-string constant that I check for, just to make this more
clear.

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