Merge lp:~geser/launchpadlib/toc into lp:launchpadlib

Proposed by Michael Bienia
Status: Merged
Merged at revision: not available
Proposed branch: lp:~geser/launchpadlib/toc
Merge into: lp:launchpadlib
Diff against target: None lines
To merge this branch: bzr merge lp:~geser/launchpadlib/toc
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+11398@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Michael Bienia (geser) wrote :

Add a Table of Contents to the API documentation XSL stylesheet (lp: #325367).
An example of the API documentation with a toc can be seen at http://www.bienia.de/tmp/apidoc.html

Revision history for this message
Curtis Hovey (sinzui) wrote :

=== modified file 'src/launchpadlib/wadl-to-refhtml.xsl'
--- src/launchpadlib/wadl-to-refhtml.xsl 2009-08-14 16:48:11 +0000
+++ src/launchpadlib/wadl-to-refhtml.xsl 2009-09-08 21:43:30 +0000
...
> @@ -413,12 +416,53 @@
> <h1><xsl:value-of select="$title" /></h1>
> <xsl:apply-templates select="wadl:doc"/>
>
> + <xsl:call-template name="table-of-contents" />
> <xsl:call-template name="top-level-collections" />
> <xsl:call-template name="entry-types" />
> </body>
> </html>
> </xsl:template>
>
> + <!-- Table of contents -->
> + <xsl:template name="table-of-contents">
> + <div id="toc">
> + <h2>Table of Contents</h2>
> + <h3>Top-level collections</h3>

> + <xsl:for-each
> + select="key('id', 'service-root-json')/wadl:param/wadl:link">
> + <xsl:sort select="../@name" />
> + <xsl:variable name="collection_id"
> + select="substring-after(@resource_type, '#')" />
> + <xsl:if test="string-length($collection_id) > 0">

This is not well-formed. The '>' should be '&gt;'. I surprissed this works.

> + <a href="#{$collection_id}">
> + <xsl:call-template name="get-title-or-id">
> + <xsl:with-param name="element" select="key('id', $collection_id)" />
> + </xsl:call-template>
> + </a><br/>
> + </xsl:if>
> + </xsl:for-each>

This is generating inline elements that are commingled with with block
elements. The browser engine knows this cannot happen and will correct this by
wrapping these in a block element of its choosing (<p>). I would have expect
this list to be a list <ul> or <ol>. At the very least, these <a> and
<br /> elements should be in a div.

> + <h3>Entry types</h3>

> + <xsl:for-each select="wadl:resource_type[
> + @id != 'service-root'
> + and @id != 'HostedFile'
> + and not(contains(@id, 'page-resource'))
> + ]">
> + <xsl:sort select="@id" />
> + <xsl:variable name="id" select="./@id"/>
> + <xsl:variable name="top_level_collections"
> + select="key('id', 'service-root-json')//@resource_type[
> + substring-after(., '#') = $id]" />
> + <xsl:if test="not($top_level_collections[contains(., $id)])">
> + <a href="#{$id}">
> + <xsl:call-template name="get-title-or-id">
> + <xsl:with-param name="element" select="." />
> + </xsl:call-template>
> + </a><br/>
> + </xsl:if>
> + </xsl:for-each>

Again I would expect an <ol> or <ul> wrapping these links in the block flow.

> + </div>
> + </xsl:template>
...

review: Needs Fixing (code)
Revision history for this message
Curtis Hovey (sinzui) wrote :

I really appreciate you doing this. I too get lost in this document. I see an error and I have a suggestion to improve the markup. I think this branch can land with a little fixing.

lp:~geser/launchpadlib/toc updated
56. By Michael Bienia

Make the toc a list (<ul>).

57. By Michael Bienia

Fix a CSS error.
Fix some HTML errors.

Revision history for this message
Michael Bienia (geser) wrote :

I've done the suggested fixes (r56).

And done also some unrelated fixes for other HTML errors (r57).
But there is still one error remaining: some <wadl:doc> elements already contain one (or more) <p>...</p> and the template for it adds another <p>...</p> around it which the W3C validator marks as an error. I didn't figure out how to only add a <p> if there is none (my XSLT/XPath knowledge is not good enough for it).

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Michael.

Thanks for this fixes. This is a welcome improvement.

We may want to consider changing <xsl:template match="wadl:doc"> to create a div since we are getting inline and block content. The other option is to create a new mode called inline-copy that ignores p,div,blockquote,table,ul,ol,dl. I think the dic option is something worth trying.

Regadless, you have fixed more than requested. This branch is fine to land. If you want to ignore the <p> in <p> problem, please report a bug so that others know about this issue.

review: Approve (code)
Revision history for this message
Michael Bienia (geser) wrote :

> Regadless, you have fixed more than requested. This branch is fine to land. If
> you want to ignore the <p> in <p> problem, please report a bug so that others
> know about this issue.

Filed as bug #426858.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/launchpadlib/wadl-to-refhtml.xsl'
--- src/launchpadlib/wadl-to-refhtml.xsl 2009-08-14 16:48:11 +0000
+++ src/launchpadlib/wadl-to-refhtml.xsl 2009-09-08 21:43:30 +0000
@@ -123,6 +123,9 @@
123 font-weight: normal;123 font-weight: normal;
124 opacity: 0.75;124 opacity: 0.75;
125 }125 }
126 .toc-link {
127 font-size: 0.85em;
128 }
126 </style>129 </style>
127 </xsl:template>130 </xsl:template>
128131
@@ -413,12 +416,53 @@
413 <h1><xsl:value-of select="$title" /></h1>416 <h1><xsl:value-of select="$title" /></h1>
414 <xsl:apply-templates select="wadl:doc"/>417 <xsl:apply-templates select="wadl:doc"/>
415418
419 <xsl:call-template name="table-of-contents" />
416 <xsl:call-template name="top-level-collections" />420 <xsl:call-template name="top-level-collections" />
417 <xsl:call-template name="entry-types" />421 <xsl:call-template name="entry-types" />
418 </body>422 </body>
419 </html>423 </html>
420 </xsl:template>424 </xsl:template>
421425
426 <!-- Table of contents -->
427 <xsl:template name="table-of-contents">
428 <div id="toc">
429 <h2>Table of Contents</h2>
430 <h3>Top-level collections</h3>
431 <xsl:for-each
432 select="key('id', 'service-root-json')/wadl:param/wadl:link">
433 <xsl:sort select="../@name" />
434 <xsl:variable name="collection_id"
435 select="substring-after(@resource_type, '#')" />
436 <xsl:if test="string-length($collection_id) > 0">
437 <a href="#{$collection_id}">
438 <xsl:call-template name="get-title-or-id">
439 <xsl:with-param name="element" select="key('id', $collection_id)" />
440 </xsl:call-template>
441 </a><br/>
442 </xsl:if>
443 </xsl:for-each>
444 <h3>Entry types</h3>
445 <xsl:for-each select="wadl:resource_type[
446 @id != 'service-root'
447 and @id != 'HostedFile'
448 and not(contains(@id, 'page-resource'))
449 ]">
450 <xsl:sort select="@id" />
451 <xsl:variable name="id" select="./@id"/>
452 <xsl:variable name="top_level_collections"
453 select="key('id', 'service-root-json')//@resource_type[
454 substring-after(., '#') = $id]" />
455 <xsl:if test="not($top_level_collections[contains(., $id)])">
456 <a href="#{$id}">
457 <xsl:call-template name="get-title-or-id">
458 <xsl:with-param name="element" select="." />
459 </xsl:call-template>
460 </a><br/>
461 </xsl:if>
462 </xsl:for-each>
463 </div>
464 </xsl:template>
465
422 <!-- Top level collections container -->466 <!-- Top level collections container -->
423 <xsl:template name="top-level-collections">467 <xsl:template name="top-level-collections">
424 <div id="top-level-collections">468 <div id="top-level-collections">
@@ -512,6 +556,7 @@
512556
513 <xsl:call-template name="custom-GETs" />557 <xsl:call-template name="custom-GETs" />
514 <xsl:call-template name="custom-POSTs" />558 <xsl:call-template name="custom-POSTs" />
559 <a href="#toc" class="toc-link">(back to Table of Contents)</a>
515 </div>560 </div>
516 </xsl:template>561 </xsl:template>
517562
@@ -626,6 +671,7 @@
626 <xsl:call-template name="standard-methods" />671 <xsl:call-template name="standard-methods" />
627 <xsl:call-template name="custom-GETs" />672 <xsl:call-template name="custom-GETs" />
628 <xsl:call-template name="custom-POSTs" />673 <xsl:call-template name="custom-POSTs" />
674 <a href="#toc" class="toc-link">(back to Table of Contents)</a>
629 </xsl:template>675 </xsl:template>
630676
631 <!-- Documentation of the default representation for an entry -->677 <!-- Documentation of the default representation for an entry -->

Subscribers

People subscribed via source and target branches