Extensions Docs Server: Enum values do not show up if enum is a type

tools/json_schema_compiler/cc_generator.py

Index: tools/json_schema_compiler/cc_generator.py
diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py
index f01479a2f612bb6c41b48a785a9de8898224ced0..4da4ed611e054de7889cf49c0b8f9f0fafc1bb94 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -130,6 +130,8 @@ class CCGenerator(object):
     elif self._cpp_type_generator.IsEnumOrEnumRef(type_):
       c.Concat(self._GenerateCreateEnumTypeValue(cpp_namespace, type_))
       c.Append()
+      c.Concat(self._GenerateGetEnumTypeFromString(cpp_namespace, type_))
+      c.Append()

[not at google - send to devlin, 2012/09/14 01:44:51]

nit: style like

(c.Concat(...)
  .Append()
  .Concat(...)
  .Append())

[cduvall, 2012/09/17 22:07:46]

Done.

     c.Substitute({'classname': classname, 'namespace': cpp_namespace})
 
     return c
@@ -220,12 +222,15 @@ class CCGenerator(object):
     c.Append('const base::Value* %(value_var)s = NULL;')
     if prop.optional:
       (c.Sblock(
-          'if (%(src)s->GetWithoutPathExpansion("%(key)s", &%(value_var)s)) {'
-        )
+          'if (%(src)s->GetWithoutPathExpansion("%(key)s", &%(value_var)s)) {')
         .Concat(self._GeneratePopulatePropertyFromValue(
             prop, value_var, dst, 'false'))
-        .Eblock('}')
-      )
+        .Eblock('}'))
+      if self._cpp_type_generator.IsEnumOrEnumRef(prop):
+        (c.Sblock('else {')

[not at google - send to devlin, 2012/09/14 01:44:51]

would be nice to generate style-guide compatible code here, like
} else {
rather than
}
else {
you might need to extend code.py to be able to do it, though. Maybe, err, Mblock
(M for Middle) or something.

[cduvall, 2012/09/17 22:07:46]

Done.

+          .Append('%%(dst)s->%%(name)s = %s;' %
+              self._cpp_type_generator.GetEnumNoneValue(prop))
+          .Eblock('}'))
     else:
       (c.Append(
           'if (!%(src)s->GetWithoutPathExpansion("%(key)s", &%(value_var)s))')
@@ -234,7 +239,13 @@ class CCGenerator(object):
             prop, value_var, dst, 'false'))
       )
     c.Append()
-    c.Substitute({'value_var': value_var, 'key': prop.name, 'src': src})
+    c.Substitute({
+      'value_var': value_var,
+      'key': prop.name,
+      'src': src,
+      'dst': dst,
+      'name': prop.unix_name
+    })
     return c
 
   def _GenerateTypeToValue(self, cpp_namespace, type_):
@@ -648,11 +659,7 @@ class CCGenerator(object):
       c.Append('%(dst)s->%(name)s' + accessor + 'push_back(enum_temp);')
       c.Eblock('}')
 
-  def _GenerateStringToEnumConversion(self,
-                                      c,
-                                      prop,
-                                      value_var,
-                                      enum_temp):
+  def _GenerateStringToEnumConversion(self, c, prop, value_var, enum_temp):
     """Appends code that converts a string to an enum.
     Leaves failure_value unsubstituted.
 
@@ -661,23 +668,21 @@ class CCGenerator(object):
     value_var: the string value that is being converted.
     enum_temp: the name used to store the temporary enum value.
     """
-    (c.Append('%s %s;' % (self._cpp_type_generator.GetCompiledType(prop),
-                          enum_temp))
-      .Append('std::string enum_as_string;')
+    c.Append('%s %s = %s;' % (self._cpp_type_generator.GetCompiledType(prop),
+                              enum_temp,
+                              self._cpp_type_generator.GetEnumNoneValue(prop)))
+    (c.Append('std::string enum_as_string;')
       .Append('if (!%s->GetAsString(&enum_as_string))' % value_var)
-      .Append('  return %(failure_value)s;')
-    )
+      .Append('  return %(failure_value)s;'))
     for i, enum_value in enumerate(
           self._cpp_type_generator.GetReferencedProperty(prop).enum_values):
-      (c.Append(
-          ('if' if i == 0 else 'else if') +
-          '(enum_as_string == "%s")' % enum_value)
-        .Append('  ' + enum_temp + ' = %s;' % (
-            self._cpp_type_generator.GetEnumValue(prop, enum_value)))
-      )
-    (c.Append('else')
-      .Append('  return %(failure_value)s;')
-    )
+      (c.Append('if (enum_as_string == "%s")' % enum_value)
+        .Append('  %s = %s;' %
+             (enum_temp,
+             self._cpp_type_generator.GetEnumValue(prop, enum_value))))
+    (c.Append('if (%s == %s)' %
+        (enum_temp, self._cpp_type_generator.GetEnumNoneValue(prop)))
+     .Append('  return %(failure_value)s;'))
 
   def _GeneratePropertyFunctions(self, param_namespace, params):
     """Generate the functions for structures generated by a property such as
@@ -721,9 +726,10 @@ class CCGenerator(object):
           self._cpp_type_generator.GetEnumValue(prop, choice.type_.name),
           'make_scoped_ptr<base::Value>(%s)' %
               self._CreateValueFromProperty(choice, choice.unix_name)))
-    (c.Eblock('}')
-      .Append('NOTREACHED();')
-      .Append('return scoped_ptr<base::Value>();')
+    (c.Append('default:')

[not at google - send to devlin, 2012/09/14 01:44:51]

so... is this a place where we legitimately don't know whether there's a NONE
value being generated? so we need to catch it in default too?

I mean, surely, we know. because we end up generating the code. question is how
much pain it is.

[cduvall, 2012/09/17 22:07:46]

Done.

+      .Append('  NOTREACHED();')
+      .Append('  return scoped_ptr<base::Value>();')
+      .Eblock('}')
       .Eblock('}')
       .Append()
       .Substitute({
@@ -749,10 +755,40 @@ class CCGenerator(object):
           '%s_%s' % (classname.upper(), enum_value.upper()),
           'scoped_ptr<base::Value>(base::Value::CreateStringValue("%s"))' %
           enum_value))
-    (c.Eblock('}')
-      .Append('NOTREACHED();')
-      .Append('return scoped_ptr<base::Value>();')
+    # We need a default case because all enum types have a *_NONE value, which
+    # is not handled in the other cases.

[not at google - send to devlin, 2012/09/14 01:44:51]

confused, can't you handle the NONE case explicitly then?

[cduvall, 2012/09/17 22:07:46]

Done.

+    (c.Append('default:')
+      .Append('  return scoped_ptr<base::Value>();')
+      .Eblock('}')
+      .Eblock('}'))
+    return c
+
+  def _GenerateGetEnumTypeFromString(self, cpp_namespace, prop):
+    """Generates GetEnumFromString() which gets an enum from its string
+    representation.
+    """
+    c = Code()
+    classname = cpp_util.Classname(schema_util.StripSchemaNamespace(prop.name))
+    (c.Append('// static')
+      .Sblock('bool GetEnumFromString(const std::string& enum_string,'

[not at google - send to devlin, 2012/09/14 01:44:51]

does it make sense to return the enum directly? We have a _NONE value generated
for enums to use as the default case. out-param style methods are so awkward.

Also, since it's on the enum type already, FromString() is probably enough.

Also, seems like this should be using GenerateStringToEnumConverstion in some
way (or GSTEC should be modified to call this).

[cduvall, 2012/09/17 22:07:46]

Done.

+              ' %(class)s* %(var)s) {'))

[not at google - send to devlin, 2012/09/14 01:44:51]

how about generating a ToString(enum) too? seems useful. There might be code
around here somewhere for that.

[cduvall, 2012/09/17 22:07:46]

Done.

+    enum_prop = self._cpp_type_generator.GetReferencedProperty(prop)
+    for i, enum_value in enumerate(
+          self._cpp_type_generator.GetReferencedProperty(prop).enum_values):
+      # This is broken up into all ifs with no else ifs because we get
+      # "fatal error C1061: compiler limit : blocks nested too deeply"
+      # on Windows.

[not at google - send to devlin, 2012/09/14 01:44:51]

haha

+      (c.Sblock('if (enum_string == "%s") {' % enum_value)
+        .Append('*%%(var)s = %s_%s;' % (classname.upper(), enum_value.upper()))
+        .Append('return true;')
+        .Eblock('}'))
+    (c.Append('return false;')
       .Eblock('}')
+      .Substitute({
+        'cpp_namespace': cpp_namespace,
+        'var': classname.lower(),
+        'class': classname
+      })
     )
     return c
 
@@ -777,9 +813,10 @@ class CCGenerator(object):
           self._cpp_type_generator.GetEnumValue(prop, enum_value),
           'scoped_ptr<base::Value>(base::Value::CreateStringValue("%s"))' %
           enum_value))
-    (c.Eblock('}')
-      .Append('NOTREACHED();')
-      .Append('return scoped_ptr<base::Value>();')
+    (c.Append('default:')

[not at google - send to devlin, 2012/09/14 01:44:51]

ditto

[cduvall, 2012/09/17 22:07:46]

Done.

+      .Append('  NOTREACHED();')
+      .Append('  return scoped_ptr<base::Value>();')
+      .Eblock('}')
       .Eblock('}')
       .Substitute({
         'cpp_namespace': cpp_namespace,