[webkit-reviews] review granted: [Bug 137295] Web Inspector: Move the computation that results in UI strings from JSC to the Web Inspector : [Attachment 239191] patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Saam Barati
<saambarati1 at gmail.com>'s request for review:
Bug 137295: Web Inspector: Move the computation that results in UI strings from
JSC to the Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=137295

Attachment 239191: patch
https://bugs.webkit.org/attachment.cgi?id=239191&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=239191&action=review


r=me

> Source/WebInspectorUI/UserInterface/Models/TypeSet.js:122
> +	   // It's implied that type Integer is contained in type Number. Don't
put 
> +	   // both 'Integer' and 'Number' into the set because this could imply
that 
> +	   // Number means to Double instead of Double|Integer.
> +	   if (typeSet.isNumber)
> +	       this._primitiveTypeNames.push("Number");
> +	   else if (typeSet.isInteger)
> +	       this._primitiveTypeNames.push("Integer");

I wonder if this could be a feature of typeSet. It's a bit weird that typeSet
responds "true" to "isInteger" for something that is, in fact, number. Or
perhaps "is" should be renamed to "contains"?