Extensions Docs Server: api_index.html, experimental.html

chrome/common/extensions/docs/server2/api_list_data_source.py

+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+
+import third_party.json_schema_compiler.model as model
+
+class APIListDataSource(object):
+  """ This class creates a list of chrome.* APIs and chrome.experimental.* APIs
+  that are used in the api_index.html and experimental.html pages.
+  """
+  def __init__(self, cache_builder, file_system, api_path, public_path):
+    self._cache = cache_builder.build(self._ListAPIs)
+    self._file_system = file_system
+    self._api_path = api_path + '/'
+    self._public_path = public_path + '/'
+
+  def _ListAPIs(self, apis):
+    api_names = [os.path.splitext(name)[0] for name in apis]

[not at google - send to devlin, 2012/07/23 12:40:24]

this is only used for lookup, so make it a set?

api_names = set(os.path.splitext(name)[0] for name in apis)

turns out that works in Python. sweet.

[cduvall, 2012/07/23 20:38:19]

Done.

+    template_names = [os.path.splitext(name)[0] for name in
+                      self._file_system.Read(

[not at google - send to devlin, 2012/07/23 12:40:24]

Use that fancy new ReadSingle method?

(or add it in this patch if it goes through first?)

also looks like python style guide
(http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#List_Comprehen...)
says that each portion of the list comprehension should go on its own line, so
looks like the result of reading from public_path will need to be assigned to
its own variable...

+                          [self._public_path]).Get()[self._public_path]]
+    unix_template_names = [model.UnixName(name) for name in template_names]
+    experimental_names = []
+    # Go through the template names, and remove all names not found in the api
+    # folder from the list. Add all experimental APIs to a separate list.
+    for name, i in zip(template_names, range(len(template_names))):

[not at google - send to devlin, 2012/07/23 12:40:24]

Perhaps

for i, template_name in enumerate(template_names):
  ...

would be a more common idiom.

[cduvall, 2012/07/23 20:38:19]

Done.

+      if unix_template_names[i] not in api_names:
+        template_names.remove(name)

[not at google - send to devlin, 2012/07/23 12:40:24]

removing stuff is O(n). Instead, add to a list like "chrome_apis" and
"experimental_apis" rather than removing from template_names?

Also don't really need that unix_template_names thing, just have

if model.UnixName(template_name) in api_names:
  if template_name.startswith('experimental'):
    experimental_apis.append(template_name)
  else:
    chrome_apis.append(template_name)

[cduvall, 2012/07/23 20:38:19]

Done.

+      elif unix_template_names[i].startswith('experimental'):
+        template_names.remove(name)
+        experimental_names.append(name)
+    return {
+      'chrome': sorted([name.replace('_', '.') for name in template_names]),
+      'experimental': sorted([name.replace('_', '.') for name in
+                              experimental_names])

[not at google - send to devlin, 2012/07/23 12:40:24]

similar comment as above, list comp "for foo in bar" thig should go on its own
line.

that said it make just as much sense to do the replacement while adding to those
list of APIs, in fact it kind of makes more sense since those are the names of
the APIs...

List comprehensions can be neat, but if avoidable...

[cduvall, 2012/07/23 20:38:19]

Done.

+    }
+
+  def __getitem__(self, key):
+    return self.get(key)
+
+  def get(self, key):
+    try:
+      return self._cache.GetFromFile(self._api_path)

[not at google - send to devlin, 2012/07/23 12:40:24]

if you added [key] at the end of here then you could do {{#api_list.chrome}}
rather than {{#api_list.list.chrome}} and likewise experimental.

[cduvall, 2012/07/23 20:38:19]

Done.

+    except Exception as e:
+      return None