Bug 109771 - Message generation should handle nested templates
Summary: Message generation should handle nested templates
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-13 16:38 PST by Anders Carlsson
Modified: 2013-02-13 17:51 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.43 KB, patch)
2013-02-13 16:42 PST, Anders Carlsson
no flags Details | Formatted Diff | Diff
Patch (10.72 KB, patch)
2013-02-13 17:05 PST, Anders Carlsson
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2013-02-13 16:38:25 PST
Message generation should handle nested templates
Comment 1 Anders Carlsson 2013-02-13 16:42:59 PST
Created attachment 188223 [details]
Patch
Comment 2 Early Warning System Bot 2013-02-13 16:57:57 PST
Comment on attachment 188223 [details]
Patch

Attachment 188223 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16527418
Comment 3 Anders Carlsson 2013-02-13 17:05:10 PST
Created attachment 188227 [details]
Patch
Comment 4 Ryosuke Niwa 2013-02-13 17:35:46 PST
Comment on attachment 188227 [details]
Patch

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

> Source/WebKit2/ChangeLog:13
> +        (template_parameters):
> +        (class_template_headers):

Maybe explain that they're mutually recursive?

> Source/WebKit2/Scripts/webkit2/messages.py:376
> +        return ([], [template_string])

Why don't we use dictionary for the return value instead so that we don't have to remember which one is which?
Also, you don't need parenthesis here.

> Source/WebKit2/Scripts/webkit2/messages.py:439
> +    headers = []

How about using set() for headers so that we don't include the same header multiple times?

> Source/WebKit2/Scripts/webkit2/messages.py:452
> +        split = type.split('::')
> +        if len(split) < 2:
> +            continue

Should we support fully qualified names like ::WTF::HashMap?
Comment 5 Anders Carlsson 2013-02-13 17:45:37 PST
Comment on attachment 188227 [details]
Patch

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

>> Source/WebKit2/ChangeLog:13
>> +        (class_template_headers):
> 
> Maybe explain that they're mutually recursive?

Only class_template_headers is recursive. I’ll add that. Come to think of it, template_parameters can just call parser.split_parameters_string - I’ll do that.

>> Source/WebKit2/Scripts/webkit2/messages.py:376
>> +        return ([], [template_string])
> 
> Why don't we use dictionary for the return value instead so that we don't have to remember which one is which?
> Also, you don't need parenthesis here.

OK!

>> Source/WebKit2/Scripts/webkit2/messages.py:439
>> +    headers = []
> 
> How about using set() for headers so that we don't include the same header multiple times?

Headers are uniqued later.

>> Source/WebKit2/Scripts/webkit2/messages.py:452
>> +            continue
> 
> Should we support fully qualified names like ::WTF::HashMap?

Nah, WTF:: is fine.
Comment 6 Anders Carlsson 2013-02-13 17:51:11 PST
Committed r142835: <http://trac.webkit.org/changeset/142835>