Issue 2306 - Add toc macro to web.adblockplus.org

2306 - Add toc macro to web.adblockplus.org

Depends on
 - 2119 adding get_page_content global function to the CMS: http://codereview.adblockplus.org/5148261828526080/
 - Corresponding changes to conversion script https://github.com/kzar/adblockplus-website-converter/commit/0acddcf307a4f1febbb6df01d633b66672990e56

[kzar, 2015-04-07 21:06:04]

Patch Set 1

[Sebastian Noack, 2015-04-13 09:58:15]

Issue 2118 only specifies the change to the migration script. The change here
isn't mentioned at all. I suggest you file a separate issue.

http://codereview.adblockplus.org/5158802752012288/diff/5629499534213120/incl...
File includes/toc.tmpl (right):

http://codereview.adblockplus.org/5158802752012288/diff/5629499534213120/incl...
includes/toc.tmpl:28: <a href="{{ page }}#{{item.anchor}}">{{item.title}}</a>
Nit: Please add whitespaces after "{{" and before "}}" for consistency.

[kzar, 2015-04-13 11:20:21]

Patch Set 2 : Made spacing more consistent.

I've created new issue for this and renamed the review etc accordingly.

http://codereview.adblockplus.org/5158802752012288/diff/5629499534213120/incl...
File includes/toc.tmpl (right):

http://codereview.adblockplus.org/5158802752012288/diff/5629499534213120/incl...
includes/toc.tmpl:28: <a href="{{ page }}#{{item.anchor}}">{{item.title}}</a>
On 2015/04/13 09:58:16, Sebastian Noack wrote:
> Nit: Please add whitespaces after "{{" and before "}}" for consistency.

Done.

[Wladimir Palant, 2015-04-13 12:00:35]

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
File includes/toc.tmpl (right):

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
includes/toc.tmpl:18: {% macro toc(page, title_tag) %}
Should be title_tag=None the way I see it, this parameter is optional.

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
includes/toc.tmpl:28: <a href="{{ page }}#{{ item.anchor }}">{{ item.title
}}</a>
This is a relative link, it won't work correctly in subdirectories for example.
I think {{ (page + '#' + item.anchor)|linkify }} should generate a proper
absolute link here.

[kzar, 2015-04-13 12:49:13]

Patch Set 3 : Addressed Wladimir's comments.

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
File includes/toc.tmpl (right):

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
includes/toc.tmpl:18: {% macro toc(page, title_tag) %}
On 2015/04/13 12:00:36, Wladimir Palant wrote:
> Should be title_tag=None the way I see it, this parameter is optional.

Done.

http://codereview.adblockplus.org/5158802752012288/diff/5724160613416960/incl...
includes/toc.tmpl:28: <a href="{{ page }}#{{ item.anchor }}">{{ item.title
}}</a>
On 2015/04/13 12:00:36, Wladimir Palant wrote:
> This is a relative link, it won't work correctly in subdirectories for
example.
> I think {{ (page + '#' + item.anchor)|linkify }} should generate a proper
> absolute link here.

Ouch, Done.

[Wladimir Palant, 2015-04-13 19:00:33]

LGTM