[Webkit-unassigned] [Bug 72835] Web Inspector: [protocol] generate C++ classes for protocol JSON named types

[bugzilla-daemon at webkit.org]

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





--- Comment #28 from Peter Rybin <peter.rybin at gmail.com>  2011-12-05 16:21:53 PST ---
> > Source/WebCore/inspector/CodeGeneratorInspector.py:110
> > +    def lower_camel_case_to_upper(str):
> > +        if len(str) > 0 and str[0].islower():
> > +            str = str[0].upper() + str[1:]
> > +        return str
> String.capitalize()?

No. According to documentation it makes all letters but first lowercase.

> > Source/WebCore/inspector/CodeGeneratorInspector.py:138
> > +        pos = 0
> > +        while pos < len(output):
> > +            output[pos] = output[pos].upper()
> > +            pos += 1
> > +        return "_".join(output)
> Why not just "_".join(output).upper()?

Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:152
> > +    def split_camel_case_(str):
> > +        output = []
> > +        pos_being = 0
> > +        pos = 1
> > +        has_oneletter = False
> > +        while pos < len(str):
> > +            if str[pos].isupper():
> > +                output.append(str[pos_being:pos].upper())
> > +                if pos - pos_being == 1:
> > +                    has_oneletter = True
> > +                pos_being = pos
> > +            pos += 1
> This looks as it were ported from COBOL. Can this be rewritten into python -- e.g., use regexps?

I don't really see how it can be re-written nicer. At least in 15 minutes.

> > Source/WebCore/inspector/CodeGeneratorInspector.py:459
> > +                        output.append("namespace ")
> > +                        output.append(json_type["id"])
> > +                        output.append(" {\n")
> Consider using interpolation for better readability.

I do use a lot of interpolation throughout the script. I'm not convinced it always gives better readability.

> > Source/WebCore/inspector/CodeGeneratorInspector.py:732
> > +        def fill_recursive(path_part, depth):
> > +            if depth <= 0 or path_part == '/':
> > +                return
> > +            fill_recursive(os.path.dirname(path_part), depth - 1)
> > +            components.append(os.path.basename(path_part))
> Does this have to be recursive? Also, consider os.path.split(). Or, given that you join them back with "/".join() below, you could as well have used a regexp.

I don't know. I have to split path into components. Given I only can get the last component per time, recursion looked pretty nice solution. I don't like to use regexp, because I'm not sure about Windows stuff.

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