Extensions Docs Server: Split apps from extensions

Extensions Docs Server: Split apps from extensions

The apps and extensions templates are now in separate directories. I didn't
customize the intros (like removing sections marked class="doc-family apps"),
because the conversion script will have to do that.

Unfortunately, it looks like the current docs system just uses a list of
non-apps APIs to tell the difference between apps and extensions APIs:
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/common/extensions/docs/js/api_page_generator.js&exact_package=chromium&q=listChromeAPI&type=cs&l=546

BUG=139897, 140251, 140270
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150159

[not at google - send to devlin, 2012-08-03 20:05:29]

This change doesn't need to be so magical. It only needs to be by convention
that there's an apps and extensions split.

What you have with the two templates, one for apps and one for extensions, looks
good.

You'll also need code in samples_data_source and api_list_data_source to expose
different sections for apps and extensions.

With that, the search box and TOC and samples list etc... all that stuff that
needs to be different can be done by changing the overall template. E.g. a
different footer for extensions and apps (or just a parameterised footer), which
bootstraps a different set of APIs depending on whether it's apps or extensions.

E.g. naively, or perhaps realistically, at the end of standard_extension_api
have

<script>
window.bootstrap.api_names = {{*api_list.extensions.chrome}}
</script>

vs

<script>
window.bootstrap.api_names = {{*api_list.apps.chrome}}
</script>

api_list_data_source needs to be able to figure out the difference, but it's not
much beyond that.

Ditto samples etc.

I wouldn't expect any changes to handler.py nor template_data_source.py.

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

http://codereview.chromium.org/10834144/diff/2001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handler.py:52: EXTENSIONS_TEMPLATE_PATH =
DOCS_PATH + '/server2/templates/extensions'
why do we need to split it like this, can't they be separate directories under
public/

[cduvall, 2012-08-03 22:37:59]

Reworked according to your comment.

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

https://chromiumcodereview.appspot.com/10834144/diff/2001/chrome/common/exten...
chrome/common/extensions/docs/server2/handler.py:52: EXTENSIONS_TEMPLATE_PATH =
DOCS_PATH + '/server2/templates/extensions'
On 2012/08/03 20:05:29, kalman wrote:
> why do we need to split it like this, can't they be separate directories under
> public/

Done.

[not at google - send to devlin, 2012-08-06 08:38:33]

look great, only a couple of comments.

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

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:22: return_dict =
{}
nit: I would have structured this like

return {
  'apps': self._GetAPIsInSubdirectory(api_names, doc_type),
  'extensions': ...
}

Declaratively clearer exactly what is being returned, so it's easier to
associate it with what's in the templates.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/branch_utility.py (right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/branch_utility.py:24: path =
path[len('apps/'):]
Is all of this necessary? The paths will be

http://developer.chrome.com/trunk/apps/storage.html vs
http://developer.chrome.com/trunk/apps/about_apps.storage

so the branch will come first?

or am I misunderstanding it?

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/integration_test.py (right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/integration_test.py:34: for filename in
os.listdir(os.path.join('templates', 'public', doc_type)):
can you do a recursive search for .html files rather than explicitly saying what
the subdirectories are?

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/articles/api_index_extensions.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index_extensions.html:6:
</p>
I think in this case we're better off passing through an is_apps parameter to
the template (sorry). These's a lot of duplication otherwise.

It's just from the template that includes it, though (which knows what it is, so
no need to change python).

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/articles/experimental_extensions.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/experimental_extensions.html:7:

Likewise, I can't imagine why this page would diverge from the extension's
version. If they have it's probably an accident.

But you'll need to pass is_apps in too.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/private/standard_apps_api.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/standard_apps_api.html:33:
api_names: {{*api_list.apps.chrome}}.concat({{*api_list.apps.experimental}}),
unfortunately... we need to stick to 80 characters here, cos it's code.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/private/standard_extensions_api.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/standard_extensions_api.html:33:
api_names:
{{*api_list.extensions.chrome}}.concat({{*api_list.extensions.experimental}}),
ditto

[cduvall, 2012-08-06 18:36:16]

See branch_utility for comments on docs paths.

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

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/api_list_data_source.py:22: return_dict =
{}
On 2012/08/06 08:38:33, kalman wrote:
> nit: I would have structured this like
> 
> return {
>   'apps': self._GetAPIsInSubdirectory(api_names, doc_type),
>   'extensions': ...
> }
> 
> Declaratively clearer exactly what is being returned, so it's easier to
> associate it with what's in the templates.

Done.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/branch_utility.py (right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/branch_utility.py:24: path =
path[len('apps/'):]
On 2012/08/06 08:38:33, kalman wrote:
> Is all of this necessary? The paths will be
> 
> http://developer.chrome.com/trunk/apps/storage.html vs
> http://developer.chrome.com/trunk/apps/about_apps.storage
> 
> so the branch will come first?
> 
> or am I misunderstanding it?

In the old docs, the branch came last. For example, the url would be
http://code.google.com/chrome/extensions/trunk/storage.html

Do you think we should change it to the branch coming first? I like it better,
but people switching from the old docs to the new docs might prefer to have it
the old way.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/integration_test.py (right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/integration_test.py:34: for filename in
os.listdir(os.path.join('templates', 'public', doc_type)):
On 2012/08/06 08:38:33, kalman wrote:
> can you do a recursive search for .html files rather than explicitly saying
what
> the subdirectories are?

Done.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/articles/api_index_extensions.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index_extensions.html:6:
</p>
On 2012/08/06 08:38:33, kalman wrote:
> I think in this case we're better off passing through an is_apps parameter to
> the template (sorry). These's a lot of duplication otherwise.
> 
> It's just from the template that includes it, though (which knows what it is,
so
> no need to change python).

Done.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/articles/experimental_extensions.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/experimental_extensions.html:7:

On 2012/08/06 08:38:33, kalman wrote:
> Likewise, I can't imagine why this page would diverge from the extension's
> version. If they have it's probably an accident.
> 
> But you'll need to pass is_apps in too.

Done.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/private/standard_apps_api.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/standard_apps_api.html:33:
api_names: {{*api_list.apps.chrome}}.concat({{*api_list.apps.experimental}}),
On 2012/08/06 08:38:33, kalman wrote:
> unfortunately... we need to stick to 80 characters here, cos it's code.

Done.

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
File
chrome/common/extensions/docs/server2/templates/private/standard_extensions_api.html
(right):

http://codereview.chromium.org/10834144/diff/7001/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/standard_extensions_api.html:33:
api_names:
{{*api_list.extensions.chrome}}.concat({{*api_list.extensions.experimental}}),
On 2012/08/06 08:38:33, kalman wrote:
> ditto

Done.

[not at google - send to devlin, 2012-08-06 22:01:23]

lgtm

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/handler.py (right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handler.py:119: real_path =
'extensions/index.html'
would be nice to do a webserver-y kind of thing and detect that these are
directories and serve index.html out of them (so like, generic handling code).
But that'd be a pain cos of all this come-from-subversion stuff.

TODO?

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/template_data_source.py (right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/template_data_source.py:115: 'FALSE':
False
hah! nice.

(would have preferred lower case though)

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

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:8:
{{?is_apps}}<p class="doc-family apps">
why not using {{:}} and having this on the same line and stuff? I would have
expected it to look like

{{?is_apps}}
..
{{:is_apps}}
..
{{/is_apps}}

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:25:
{{:}}
getting hard to read without explicitly naming what these are, so like, make it
{{:is_apps}}

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/templates/articles/experimental.html
(right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/experimental.html:2:
{{^is_apps}}<div class="doc-family extensions">
same comments as before

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/templates/private/footer.html
(right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/footer.html:14:
window.bootstrap = {
you could move the checks inside the { so that branchInfo (and any other future
bootstrap data) isn't repeated?

[not at google - send to devlin, 2012-08-06 22:02:00]

... and just checking, the build.py stuff definitely works?

[cduvall, 2012-08-06 22:07:51]

On 2012/08/06 22:02:00, kalman wrote:
> ... and just checking, the build.py stuff definitely works?

It worked last time I checked. I'll make sure though.

[cduvall, 2012-08-06 22:25:45]

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/handler.py (right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/handler.py:119: real_path =
'extensions/index.html'
On 2012/08/06 22:01:24, kalman wrote:
> would be nice to do a webserver-y kind of thing and detect that these are
> directories and serve index.html out of them (so like, generic handling code).
> But that'd be a pain cos of all this come-from-subversion stuff.
> 
> TODO?

Done.

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/template_data_source.py (right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/template_data_source.py:115: 'FALSE':
False
On 2012/08/06 22:01:24, kalman wrote:
> hah! nice.
> 
> (would have preferred lower case though)

Done.

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

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:8:
{{?is_apps}}<p class="doc-family apps">
On 2012/08/06 22:01:24, kalman wrote:
> why not using {{:}} and having this on the same line and stuff? I would have
> expected it to look like
> 
> {{?is_apps}}
> ..
> {{:is_apps}}
> ..
> {{/is_apps}}

Done.

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/api_index.html:25:
{{:}}
On 2012/08/06 22:01:24, kalman wrote:
> getting hard to read without explicitly naming what these are, so like, make
it
> {{:is_apps}}

Done.

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/templates/articles/experimental.html
(right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/articles/experimental.html:2:
{{^is_apps}}<div class="doc-family extensions">
On 2012/08/06 22:01:24, kalman wrote:
> same comments as before

Done.

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
File chrome/common/extensions/docs/server2/templates/private/footer.html
(right):

http://codereview.chromium.org/10834144/diff/4007/chrome/common/extensions/do...
chrome/common/extensions/docs/server2/templates/private/footer.html:14:
window.bootstrap = {
On 2012/08/06 22:01:24, kalman wrote:
> you could move the checks inside the { so that branchInfo (and any other
future
> bootstrap data) isn't repeated?

Done.