Bug 136606 - Web Inspector: disambiguate integral and real number primitive types in the protocol
Summary: Web Inspector: disambiguate integral and real number primitive types in the p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brian Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-09-06 17:32 PDT by Brian Burg
Modified: 2014-09-11 21:53 PDT (History)
6 users (show)

See Also:

Attachments
Patch (80.27 KB, patch)
2014-09-09 22:55 PDT, Brian Burg 		no flags Details | Formatted Diff | Diff
Patch (79.37 KB, patch)
2014-09-10 15:22 PDT, Brian Burg 		no flags Details | Formatted Diff | Diff
Patch (79.39 KB, patch)
2014-09-11 19:16 PDT, Brian Burg 		timothy: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2014-09-06 17:32:18 PDT 
Misc cleanup post-code generator rewrite.
Comment 1 Radar WebKit Bug Importer 2014-09-06 17:32:28 PDT 
<rdar://problem/18258060>
Comment 2 Brian Burg 2014-09-09 22:55:20 PDT 
Created attachment 237880 [details]
Patch
Comment 3 Timothy Hatcher 2014-09-10 08:11:44 PDT 
Comment on attachment 237880 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237880&action=review

Looks good, but I think the new names aren't great.

> Source/JavaScriptCore/inspector/InspectorValues.h:62
> +        Real,
> +        Integral,

I think Double and Integer would be better here and throughout this patch. Even if that would be less accurate, it does tend to be the names we use in WebKit and JavaScript.
Comment 4 Brian Burg 2014-09-10 15:22:30 PDT 
Created attachment 237908 [details]
Patch
Comment 5 Timothy Hatcher 2014-09-11 00:12:12 PDT 
Comment on attachment 237908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237908&action=review

> Source/JavaScriptCore/inspector/InspectorValues.h:278
> +    void pushIntegral(int);
> +    void pushReal(double);

Missed these (and callers)?
Comment 6 Brian Burg 2014-09-11 11:32:42 PDT 
Comment on attachment 237908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237908&action=review

>> Source/JavaScriptCore/inspector/InspectorValues.h:278
>> +    void pushReal(double);
> 
> Missed these (and callers)?

Oops, grep fail.
Comment 7 Brian Burg 2014-09-11 19:16:31 PDT 
Created attachment 238007 [details]
Patch
Comment 8 Brian Burg 2014-09-11 21:53:42 PDT 
Committed r173554: <http://trac.webkit.org/changeset/173554>