Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPR#7352,7353: ocamldoc, better paragraphs in html #804

Merged
merged 2 commits into from
Mar 15, 2017

Conversation

Octachron
Copy link
Member

@Octachron Octachron commented Sep 11, 2016

Companion PR to #802, this PR makes the generation of paragraph <p> tags in ocamldoc html backend much more regular.

In particular, it fixes the problem of untagged text reported in MPR#7353 and avoid emitting space-only paragraph as reported in MPR#7352. See www.polychoron.fr/ocaml-nonmanual/pr804, for example of the improved html output. Paragraph.html showcases the general improvment with respect to MPR#7353 (and have been added to ocamldoc testsuite) and Astring.html is directly the example reported in MPR#7352 by @dbuenzli.

Before this PR, ocamldoc mainly let the handling of the paragraph tags to the care of the browser: it only emitted opening <p> tag at new line, and never closed them. Consequently, the first text element of comment were never enclosed in a paragraph tag. Moreover, for each comment containing elements that cannot be contained in a paragraph (e.g. a list), the next paragraph was also not enclosed in a paragraph tag.

This PR implements a more coherent handling of paragraph within the html generator, opening and closing the corresponding paragraph. As a bonus, the new code also avoids to emits space only paragraph. However, currently, space-only paragraphs are recognized using the trim function, and would work only with ascii spaces and new lines.

Note that the style of the generated documentation have been partially adapted, but more tweaks would be needed depending on the potential merge of #802.

<br>
<b>Author(s):</b> : Florian Angeletti<br>
<b>Version:</b> : 1<br>
</div>
Copy link
Contributor

@dbuenzli dbuenzli Sep 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these <br> are gone from the other PR ? Somehow all this should also be put in a <p>.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean the documentation tags,

Author(s): : Florian Angeletti<br>
Version: : 1

then yes, all the <br> will be gone once combined with the other pr, and this fragment will be replaced by

<ul class="info-attributes"> 
<li> <b>Author(s):</b> : Florian Angeletti </li>
<li> <b>Version:</b> : 1 </li>
</ul>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok cool. I'm personnaly not very fond of tags and do not use them except for @raise but note that alternatively this could be replaced by a definition list.

<dl class="info-attributes">
 <dt>Author(s)<dt><dd>Florian Angeletti</dd>
 ....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(However they are not that convenient to style so it may not be such a good idea)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After taking the time to look at this question, the semantic of description list is indeed a better fit , but the default rendering seems quite opiniated and not that easy to style.

For instance, going back to the current rendering for documentation tags seems to require fiddling with either floating attributes or ::after pseudo-elements. I also could not find a simple way to handle the possibility to have multiple <dd> descriptions by <dt> data term.

Consequently, I am leaning towards keeping the unordered list for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's reasonable.

@xavierleroy
Copy link
Contributor

A few months later, where are we with this PR and its companion #802 ?

This commit replaces most of the use of <br> tags in ocamldoc html
backend by more meaningful tags, in order to improve the themability
of the generated html code.
@Octachron Octachron force-pushed the ocamldoc_regular_<p> branch 2 times, most recently from 591b859 to cf51bd4 Compare March 15, 2017 16:33
@Octachron Octachron merged commit 209d916 into ocaml:trunk Mar 15, 2017
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request Dec 17, 2021
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
Fix interf graph for exceptional path.
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants