Convert snakecase enum names to camelcase when stringified.

Convert snakecase enum names to camelcase when stringified.

This cl adds a new attribute to the namespace node "camel_case_enum_to_string".

This addresses the need for:
- native enums to use snake case (e.g. LOAD_COMPLETE)
- stringified values (used in js) to be camel cased (e.g. loadComplete).



BUG=309681
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270623

[David Tseng, 2014-05-13 17:10:00]

Please have a look at the changes in the python scripts. I will rename the new
stringified js values if this part looks fine with you. Thanks.

[not at google - send to devlin, 2014-05-13 18:36:34]

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:896: name = re.sub("_\S",
I don't know the re library well enough to tell what this means. Generally I
like to avoid use of re because it's rather esoteric, and simple mechanical
operations like this are achievable using standard string operations.

could you add a comment? what transformation are you making? why?

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:442: compiler_options =
{'camel_case_enum_to_string': node.value}
this means that an API can't specify more than one compiler option.

I suggest changing compiler_options to be initialised to {}, use [] syntax to
set the option, then up on line 421 where you set the compiler options instead
write "compiler_options or None" to maintain the existing behaviour where
specifying no compiler options means it's None.

[David Tseng, 2014-05-13 20:10:17]

PTAL. Still to start the rename in js but I assume the new compiler option is
fine, so I'll make sure all bots pass during my renaming pass.

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:896: name = re.sub("_\S",
On 2014/05/13 18:36:35, kalman wrote:
> I don't know the re library well enough to tell what this means. Generally I
> like to avoid use of re because it's rather esoteric, and simple mechanical
> operations like this are achievable using standard string operations.
> 
> could you add a comment? what transformation are you making? why?

I re-wrote this without re. Hopefully that's clearer.

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/20001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:442: compiler_options =
{'camel_case_enum_to_string': node.value}
On 2014/05/13 18:36:35, kalman wrote:
> this means that an API can't specify more than one compiler option.
> 
> I suggest changing compiler_options to be initialised to {}, use [] syntax to
> set the option, then up on line 421 where you set the compiler options instead
> write "compiler_options or None" to maintain the existing behaviour where
> specifying no compiler options means it's None.

Done.

[not at google - send to devlin, 2014-05-13 20:18:22]

thanks.

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:894: if 'camel_case_enum_to_string'
in self._namespace.compiler_options:
you might want to move this camel-casing logic here:

https://code.google.com/p/chromium/codesearch#chromium/src/tools/json_schema_...

close to the code that converts to the "unix name" of a function (I guess this
is snake case?).

somebody else might need it, and that's a consistent place to put it.

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:901: camel += snake[i + 1].upper()
+= on strings is expensive because it creates a new string each time. better to
have a list and then ''.join it at the end.

[David Tseng, 2014-05-13 23:16:13]

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
File tools/json_schema_compiler/cc_generator.py (right):

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:894: if 'camel_case_enum_to_string'
in self._namespace.compiler_options:
On 2014/05/13 20:18:22, kalman wrote:
> you might want to move this camel-casing logic here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/json_schema_...
> 
> close to the code that converts to the "unix name" of a function (I guess this
> is snake case?).
> 
> somebody else might need it, and that's a consistent place to put it.

Done.

https://codereview.chromium.org/273323002/diff/40001/tools/json_schema_compil...
tools/json_schema_compiler/cc_generator.py:901: camel += snake[i + 1].upper()
On 2014/05/13 20:18:22, kalman wrote:
> += on strings is expensive because it creates a new string each time. better
to
> have a list and then ''.join it at the end.

Done.

[not at google - send to devlin, 2014-05-13 23:45:47]

final round I think.. sorry.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:440: compiler_options =
{'implemented_in': node.value}
you need the [] treatment here too

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:413: 
only 1 blank line

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:491: def SnakeToCamel(snake):
could you write some unit tests? see model_test.py.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:496: if snake[i] == "_" and (i + 1) <
len(snake):
single quotes

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:498: i = i + 2
+= 2

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:501: i = i + 1
+= 1

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:502: return ''.join(camel)
the above said, it might be simpler (and more efficient) to split on '_',
uppercase each part (except the first), then join again.

[David Tseng, 2014-05-14 02:16:14]

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:440: compiler_options =
{'implemented_in': node.value}
On 2014/05/13 23:45:47, kalman wrote:
> you need the [] treatment here too

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:413: 
On 2014/05/13 23:45:47, kalman wrote:
> only 1 blank line

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:491: def SnakeToCamel(snake):
On 2014/05/13 23:45:47, kalman wrote:
> could you write some unit tests? see model_test.py.

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:496: if snake[i] == "_" and (i + 1) <
len(snake):
On 2014/05/13 23:45:47, kalman wrote:
> single quotes

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:498: i = i + 2
On 2014/05/13 23:45:47, kalman wrote:
> += 2

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:501: i = i + 1
On 2014/05/13 23:45:47, kalman wrote:
> += 1

Done.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:502: return ''.join(camel)
On 2014/05/13 23:45:47, kalman wrote:
> the above said, it might be simpler (and more efficient) to split on '_',
> uppercase each part (except the first), then join again.

Sounds like you'd still end up with some kind of loop construct.

I'm going to leave as is since split is likely to take as much time as iterating
through the string. Can change if you feel strongly. Also, note that this code
path currently only gets hit for ax_enums.idl.

[not at google - send to devlin, 2014-05-14 16:18:14]

lgtm

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:502: return ''.join(camel)
On 2014/05/14 02:16:14, David Tseng wrote:
> On 2014/05/13 23:45:47, kalman wrote:
> > the above said, it might be simpler (and more efficient) to split on '_',
> > uppercase each part (except the first), then join again.
> 
> Sounds like you'd still end up with some kind of loop construct.
> 
> I'm going to leave as is since split is likely to take as much time as
iterating
> through the string. Can change if you feel strongly. Also, note that this code
> path currently only gets hit for ax_enums.idl.

Actually split is likely to be much faster. Iterating over strings is super slow
in Python and the native string operations are all implemented in native code.

That said, given this code isn't on the critical path, it doesn't really matter.
What you have is fine.

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:413: 
no blank line

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:489: 
extra blank line

(the pep8 rule is 2 blank lines between top-level functions, 1 blank line
between methods)

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
File tools/json_schema_compiler/model_test.py (right):

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model_test.py:127: 'FOO_bar': 'FOOBar',
even though the result is uninterested, please also include a value with a
leading and/or trailing _ to make sure the result is at least defined to be
something (doesn't really matter what it is).

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model_test.py:130:
self.assertEquals(expectations[name], model.CamelName(name))
slightly nicer:

for expected, testcase in expectations.iteritems():
  self.assertEquals(expected, model.CamelCase(testcase))

[David Tseng, 2014-05-14 18:11:30]

PTAL

I've uploaded two patchsets. The first addresses your comments; I've gone ahead
and changed the impl to split as you suggested (which is also a little cleaner).

The second patchset changes cpp_omit_enum_value to cpp_enum_prefix_override.
This has the nice property of cleaning up our enum prefixes when passed to the
automation API but also retains the proper cpp convetion of prefixing with the
namespace. The problem here was the prefix didn't match with the namespace, but
I am at this point not looking to change all those enum references.

I can do patchset two in a separate change, but I figured since we're here
already, you should take a look.

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/100001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:502: return ''.join(camel)
On 2014/05/14 16:18:14, kalman wrote:
> On 2014/05/14 02:16:14, David Tseng wrote:
> > On 2014/05/13 23:45:47, kalman wrote:
> > > the above said, it might be simpler (and more efficient) to split on '_',
> > > uppercase each part (except the first), then join again.
> > 
> > Sounds like you'd still end up with some kind of loop construct.
> > 
> > I'm going to leave as is since split is likely to take as much time as
> iterating
> > through the string. Can change if you feel strongly. Also, note that this
code
> > path currently only gets hit for ax_enums.idl.
> 
> Actually split is likely to be much faster. Iterating over strings is super
slow
> in Python and the native string operations are all implemented in native code.
> 
> That said, given this code isn't on the critical path, it doesn't really
matter.
> What you have is fine.

Thanks; good to know.

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:413: 
On 2014/05/14 16:18:14, kalman wrote:
> no blank line

Done.

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model.py:489: 
On 2014/05/14 16:18:14, kalman wrote:
> extra blank line
> 
> (the pep8 rule is 2 blank lines between top-level functions, 1 blank line
> between methods)

Done.

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
File tools/json_schema_compiler/model_test.py (right):

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model_test.py:127: 'FOO_bar': 'FOOBar',
On 2014/05/14 16:18:14, kalman wrote:
> even though the result is uninterested, please also include a value with a
> leading and/or trailing _ to make sure the result is at least defined to be
> something (doesn't really matter what it is).

Done.

https://codereview.chromium.org/273323002/diff/120001/tools/json_schema_compi...
tools/json_schema_compiler/model_test.py:130:
self.assertEquals(expectations[name], model.CamelName(name))
On 2014/05/14 16:18:14, kalman wrote:
> slightly nicer:
> 
> for expected, testcase in expectations.iteritems():
>   self.assertEquals(expected, model.CamelCase(testcase))

Done.

(but note that the expected/testcase are flipped in this case with expectation
as the value, testcase as the key).
This is in line with the previous test.

[not at google - send to devlin, 2014-05-14 23:43:42]

lgtm

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:321: for property_name in (
should fit on 1 line now

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:324: result[property_name] = True
hm I wonder if the "default" value for IDL attributes is True? that would mean
that the code could really stay the same, except instead of =True here, it would
be the result from GetProperty().

seems more consistent than doing the same check twice.

[David Tseng, 2014-05-15 01:12:36]

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
File tools/json_schema_compiler/idl_schema.py (right):

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:321: for property_name in (
On 2014/05/14 23:43:43, kalman wrote:
> should fit on 1 line now

Not anymore with the below change.

https://codereview.chromium.org/273323002/diff/220001/tools/json_schema_compi...
tools/json_schema_compiler/idl_schema.py:324: result[property_name] = True
On 2014/05/14 23:43:43, kalman wrote:
> hm I wonder if the "default" value for IDL attributes is True? that would mean
> that the code could really stay the same, except instead of =True here, it
would
> be the result from GetProperty().
> 
> seems more consistent than doing the same check twice.

Done (after verifying True gets returned for properties).

[David Tseng, 2014-05-15 01:12:58]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-05-15 01:14:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtseng@chromium.org/273323002/240001

[commit-bot: I haz the power, 2014-05-15 03:14:46]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-15 08:04:04]

Change committed as 270623