Extensions Docs Server: api_index.html, experimental.html

Extensions Docs Server: api_index.html, experimental.html

Implemented the api_index.html and experimental.html pages using an
api_list_data_source.

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

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

[cduvall, 2012-07-20 21:51:29]

Some more special pages.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
Remove the h1 from the file because it is put in later in standard_article.html.

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

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_data_source.py:15: """ This class
fetches and loads JSON APIs from the FileSystem passed in with
see comment in other code review. I was being silly. You can leave this the way
it was, leading-space-wise.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/api_list_data_source.py (right):

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:20: api_names =
[os.path.splitext(name)[0] for name in apis]
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.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:22:
self._file_system.Read(
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...

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:28: for name, i in
zip(template_names, range(len(template_names))):
Perhaps

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

would be a more common idiom.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:30:
template_names.remove(name)
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)

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:37:
experimental_names])
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...

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/api_list_data_source.py:45: return
self._cache.GetFromFile(self._api_path)
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.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
On 2012/07/20 21:51:29, cduvall wrote:
> Remove the h1 from the file because it is put in later in
standard_article.html.

If it's here anyway why remove it?

If we do for some reason, can we arrange it so that we don't? We should avoid
changing the document if possible.

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
File chrome/common/extensions/docs/server2/templates/articles/api_index.html
(right):

https://chromiumcodereview.appspot.com/10815042/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:21:
{{#api_list.list.chrome}}
what's with the "list"?

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

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_data_source.py:15: """ This class
fetches and loads JSON APIs from the FileSystem passed in with
On 2012/07/23 12:40:24, kalman wrote:
> see comment in other code review. I was being silly. You can leave this the
way
> it was, leading-space-wise.

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/api_list_data_source.py (right):

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:20: api_names =
[os.path.splitext(name)[0] for name in apis]
On 2012/07/23 12:40:24, kalman wrote:
> 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.

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:28: for name, i in
zip(template_names, range(len(template_names))):
On 2012/07/23 12:40:24, kalman wrote:
> Perhaps
> 
> for i, template_name in enumerate(template_names):
>   ...
> 
> would be a more common idiom.

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:30:
template_names.remove(name)
On 2012/07/23 12:40:24, kalman wrote:
> 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)

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:37:
experimental_names])
On 2012/07/23 12:40:24, kalman wrote:
> 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...

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:45: return
self._cache.GetFromFile(self._api_path)
On 2012/07/23 12:40:24, kalman wrote:
> 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.

Done.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
On 2012/07/23 12:40:24, kalman wrote:
> On 2012/07/20 21:51:29, cduvall wrote:
> > Remove the h1 from the file because it is put in later in
> standard_article.html.
> 
> If it's here anyway why remove it?
> 
> If we do for some reason, can we arrange it so that we don't? We should avoid
> changing the document if possible.

I did this so in the template, the title could go above the TOC. I'll take this
out and you can look at the page and see if you're alright with where the TOC
is.

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/templates/articles/api_index.html
(right):

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:21:
{{#api_list.list.chrome}}
On 2012/07/23 12:40:24, kalman wrote:
> what's with the "list"?

Fixed with change in api_list_data_source

[not at google - send to devlin, 2012-07-23 23:37:43]

lgtm

nice one, I guess the <h1> thing could be done in a separate patch if we want to
go ahead with that?

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

http://codereview.chromium.org/10815042/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
On 2012/07/23 20:38:19, cduvall wrote:
> On 2012/07/23 12:40:24, kalman wrote:
> > On 2012/07/20 21:51:29, cduvall wrote:
> > > Remove the h1 from the file because it is put in later in
> > standard_article.html.
> > 
> > If it's here anyway why remove it?
> > 
> > If we do for some reason, can we arrange it so that we don't? We should
avoid
> > changing the document if possible.
> 
> I did this so in the template, the title could go above the TOC. I'll take
this
> out and you can look at the page and see if you're alright with where the TOC
> is.

Ah I see. Is it not enough for the TOC to go over on the side like it does in
the API docs?

http://codereview.chromium.org/10815042/diff/7003/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

http://codereview.chromium.org/10815042/diff/7003/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
doesn't look like it's been taken out though..

[cduvall, 2012-07-23 23:59:54]

https://chromiumcodereview.appspot.com/10815042/diff/7003/chrome/common/exten...
File chrome/common/extensions/docs/server2/intro_data_source.py (right):

https://chromiumcodereview.appspot.com/10815042/diff/7003/chrome/common/exten...
chrome/common/extensions/docs/server2/intro_data_source.py:21: intro =
re.sub('<h1.*>(.+)</h1>', '', intro, 1)
On 2012/07/23 23:37:43, kalman wrote:
> doesn't look like it's been taken out though..

Oopsies, sorry about that.