Docserver: Display enum value descriptions in API docs.

Docserver: Display enum value descriptions in API docs.

This modifies the json schema to allow both primitive types and
dictionaries with properties "name" and optional "description" for enum
values.

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

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

[not at google - send to devlin, 2013-10-28 18:00:25]

Thanks for doing this.

https://codereview.chromium.org/39113003/diff/700001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/private/type.html (right):

https://codereview.chromium.org/39113003/diff/700001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/type.html:37: {{/enum_values}}
this is identical to the content in property.html. Add a new partial like
enum_descriptions.

https://codereview.chromium.org/39113003/diff/700001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/json_schema.js (right):

https://codereview.chromium.org/39113003/diff/700001/chrome/renderer/resource...
chrome/renderer/resources/extensions/json_schema.js:320: if (instance ===
(schema.enum[i].name || schema.enum[i]))
add function like enumToString; instance == enumToString(...);
schema.enum.map(enumToString)...

incidentally, since this file is commonjs now (and just hasn't been converted
yet) you can use a normal location function.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
File components/json_schema/json_schema_validator.cc (right):

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:198: const
base::DictionaryValue* value_dict;
= NULL

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:214: return false;
rather than the GetAsDictionary above, include TYPE_DICTIONARY in the switch
here.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:494: }
likewise

https://codereview.chromium.org/39113003/diff/700001/components/test/data/jso...
File components/test/data/json_schema/enum_schema.json (right):

https://codereview.chromium.org/39113003/diff/700001/components/test/data/jso...
components/test/data/json_schema/enum_schema.json:2: "enum": ["foo", 42,
{"name": false}]
test a description here too?

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
File tools/json_schema_compiler/h_generator.py (right):

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/h_generator.py:147:
c.Append(self._type_helper.GetEnumValue(type_, value['name']) + ',')
maybe GetEnumValue should take the enum not enum['name']

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

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:300: sys.exit('Did not process %s %s' %
(child.cls, child))
sys.exit to signal errors!? (aware that other places in the file do this; but
this would be what exceptions are for).

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:148: self.enum_values.append(enum_value)
Rather than users of Model have to access enum['name'] and presumably
enum['description'] I'd rather there be an Enum class. IDL doesn't deal in
dict-style properties; it deals with well-defined structures.

Unfortunately there's already an enum (_Enum) class for something entirely
different, but you can rename that easily enough. Or, call this EnumValue.

[Sam McNally, 2013-10-29 00:39:02]

https://codereview.chromium.org/39113003/diff/700001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/private/type.html (right):

https://codereview.chromium.org/39113003/diff/700001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/type.html:37: {{/enum_values}}
On 2013/10/28 18:00:25, kalman wrote:
> this is identical to the content in property.html. Add a new partial like
> enum_descriptions.

Done.

https://codereview.chromium.org/39113003/diff/700001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/json_schema.js (right):

https://codereview.chromium.org/39113003/diff/700001/chrome/renderer/resource...
chrome/renderer/resources/extensions/json_schema.js:320: if (instance ===
(schema.enum[i].name || schema.enum[i]))
On 2013/10/28 18:00:25, kalman wrote:
> add function like enumToString; instance == enumToString(...);
> schema.enum.map(enumToString)...
> 
> incidentally, since this file is commonjs now (and just hasn't been converted
> yet) you can use a normal location function.

Done.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
File components/json_schema/json_schema_validator.cc (right):

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:198: const
base::DictionaryValue* value_dict;
On 2013/10/28 18:00:25, kalman wrote:
> = NULL

Done.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:214: return false;
On 2013/10/28 18:00:25, kalman wrote:
> rather than the GetAsDictionary above, include TYPE_DICTIONARY in the switch
> here.

I think I need to keep it before the switch so the name attribute is checked.

https://codereview.chromium.org/39113003/diff/700001/components/test/data/jso...
File components/test/data/json_schema/enum_schema.json (right):

https://codereview.chromium.org/39113003/diff/700001/components/test/data/jso...
components/test/data/json_schema/enum_schema.json:2: "enum": ["foo", 42,
{"name": false}]
On 2013/10/28 18:00:25, kalman wrote:
> test a description here too?

Done.

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
File tools/json_schema_compiler/h_generator.py (right):

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/h_generator.py:147:
c.Append(self._type_helper.GetEnumValue(type_, value['name']) + ',')
On 2013/10/28 18:00:25, kalman wrote:
> maybe GetEnumValue should take the enum not enum['name']

Done.

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

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/idl_schema.py:300: sys.exit('Did not process %s %s' %
(child.cls, child))
On 2013/10/28 18:00:25, kalman wrote:
> sys.exit to signal errors!? (aware that other places in the file do this; but
> this would be what exceptions are for).

Done.

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
File tools/json_schema_compiler/model.py (right):

https://codereview.chromium.org/39113003/diff/700001/tools/json_schema_compil...
tools/json_schema_compiler/model.py:148: self.enum_values.append(enum_value)
On 2013/10/28 18:00:25, kalman wrote:
> Rather than users of Model have to access enum['name'] and presumably
> enum['description'] I'd rather there be an Enum class. IDL doesn't deal in
> dict-style properties; it deals with well-defined structures.
> 
> Unfortunately there's already an enum (_Enum) class for something entirely
> different, but you can rename that easily enough. Or, call this EnumValue.

Done.

[not at google - send to devlin, 2013-10-29 16:25:16]

lgtm

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
File components/json_schema/json_schema_validator.cc (right):

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:214: return false;
On 2013/10/29 00:39:02, Sam McNally wrote:
> On 2013/10/28 18:00:25, kalman wrote:
> > rather than the GetAsDictionary above, include TYPE_DICTIONARY in the switch
> > here.
> 
> I think I need to keep it before the switch so the name attribute is checked.

Ah right. Could you add a comment ("sometimes the enum declaration is a
dictionary with the enum value under "name")? In fact, since there is code below
which does basically the same thing, you could pull that out into a small
function.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:494: }
On 2013/10/28 18:00:25, kalman wrote:
> likewise

(still applicable)

https://codereview.chromium.org/39113003/diff/970002/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/39113003/diff/970002/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:329:
dst_dict['enum_values'] = []
dst_dict['enum_values'] = [{'name': value.name,
                            'description': value.description}
                           for value in type_.enum_values]

[Sam McNally, 2013-10-30 03:08:36]

+jochen for components/test/data/json_schema/enum_schema.json

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
File components/json_schema/json_schema_validator.cc (right):

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:214: return false;
On 2013/10/29 16:25:16, kalman wrote:
> On 2013/10/29 00:39:02, Sam McNally wrote:
> > On 2013/10/28 18:00:25, kalman wrote:
> > > rather than the GetAsDictionary above, include TYPE_DICTIONARY in the
switch
> > > here.
> > 
> > I think I need to keep it before the switch so the name attribute is
checked.
> 
> Ah right. Could you add a comment ("sometimes the enum declaration is a
> dictionary with the enum value under "name")? In fact, since there is code
below
> which does basically the same thing, you could pull that out into a small
> function.

Done.

https://codereview.chromium.org/39113003/diff/700001/components/json_schema/j...
components/json_schema/json_schema_validator.cc:494: }
On 2013/10/29 16:25:16, kalman wrote:
> On 2013/10/28 18:00:25, kalman wrote:
> > likewise
> 
> (still applicable)

Done (before the switch for the same reason).

https://codereview.chromium.org/39113003/diff/970002/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/39113003/diff/970002/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:329:
dst_dict['enum_values'] = []
On 2013/10/29 16:25:16, kalman wrote:
> dst_dict['enum_values'] = [{'name': value.name,
>                             'description': value.description}
>                            for value in type_.enum_values]

Done.

[jochen (gone - plz use gerrit), 2013-10-30 08:54:54]

c/t/d/json_schema lgtm

[commit-bot: I haz the power, 2013-10-30 22:37:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/1500001

[commit-bot: I haz the power, 2013-10-31 01:36:45]

Retried try job too often on win_rel for step(s) browser_tests,
chrome_frame_net_tests, chrome_frame_tests, chrome_frame_unittests,
content_browsertests, mini_installer_test, nacl_integration,
sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-10-31 01:40:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/1500001

[commit-bot: I haz the power, 2013-10-31 03:59:15]

Change committed as 231982

[commit-bot: I haz the power, 2013-10-31 07:18:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/1810001

[commit-bot: I haz the power, 2013-10-31 10:21:48]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-10-31 11:23:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/1810001

[commit-bot: I haz the power, 2013-10-31 13:43:14]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-10-31 23:17:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/2110001

[commit-bot: I haz the power, 2013-10-31 23:17:28]

Failed to apply patch for chrome/common/extensions/docs/server2/app.yaml:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/common/extensions/docs/server2/app.yaml
  Hunk #1 FAILED at 1.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/docs/server2/app.yaml.rej

Patch:       chrome/common/extensions/docs/server2/app.yaml
Index: chrome/common/extensions/docs/server2/app.yaml
diff --git a/chrome/common/extensions/docs/server2/app.yaml
b/chrome/common/extensions/docs/server2/app.yaml
index
da6d5fe5c54bcac885d8c0b8b7a448f51168c926..c31cc064d0fc190d1de6318dd887747ebc6c52b4
100644
--- a/chrome/common/extensions/docs/server2/app.yaml
+++ b/chrome/common/extensions/docs/server2/app.yaml
@@ -1,5 +1,5 @@
 application: chrome-apps-doc
-version: 2-32-10
+version: 2-33-0
 runtime: python27
 api_version: 1
 threadsafe: false

[commit-bot: I haz the power, 2013-11-01 00:14:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sammc@chromium.org/39113003/2210001

[commit-bot: I haz the power, 2013-11-01 05:27:29]

Change committed as 232297