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

docs: make jsdoc generate anchor names so ToC links work #4471

Merged
merged 5 commits into from Jul 14, 2017

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Jul 7, 2017

No description provided.

Copy link
Member Author

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Added some inline comments explaining the changes. Most the ToC links in guides now work.

@@ -35,6 +35,7 @@
},
"plugins": ["plugins/markdown"],
"markdown": {
"tags": ["example"]
"tags": ["example"],
"idInHeadings": true
Copy link
Member Author

Choose a reason for hiding this comment

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

The way that marked/jsdoc add anchor links isn't quite compatible with GFM, so, it warranted the changes in fix-api-docs.js below.

{find: /(\<h[1-6] id="(?:.*)?)video-js(.*)?"\>/g, replace: '$1videojs$2">'},
{find: /(\<h[1-6] id="(?:.*)?)don-t(.*)?"\>/g, replace: '$1dont$2">'},
{find: /(\<h[1-6] id="(?:.*)?)node-js(.*)?"\>/g, replace: '$1nodejs$2">'},
{find: /(\<h[1-6] id="(?:.*)?)vtt-js(.*)?"\>/g, replace: '$1vttjs$2">'},
Copy link
Member Author

Choose a reason for hiding this comment

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

headings with a period get a dash in jsdoc but not in GFM

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably should move some of these comments directly into the code.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs changes, but I feel obliged to post this... https://stackoverflow.com/a/1732454

{find: /(\<h[1-6] id="(?:.*)?)node-js(.*)?"\>/g, replace: '$1nodejs$2">'},
{find: /(\<h[1-6] id="(?:.*)?)vtt-js(.*)?"\>/g, replace: '$1vttjs$2">'},
{find: /(\<h[1-6] id=")-(.*)("\>)/g, replace: '$1$2$3'},
{find: /(\<h[1-6] id=")(.*)-("\>)/g, replace: '$1$2$3'},
Copy link
Member Author

Choose a reason for hiding this comment

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

headings with backticks get a dash in there. Need two rules to get those that start and end with them.

{find: /(\<h[1-6] id="(?:.*)?)vtt-js(.*)?"\>/g, replace: '$1vttjs$2">'},
{find: /(\<h[1-6] id=")-(.*)("\>)/g, replace: '$1$2$3'},
{find: /(\<h[1-6] id=")(.*)-("\>)/g, replace: '$1$2$3'},
{find: /(\<h[1-6] id=".*)-docs-guides-.*-md("\>)/g, replace: '$1$2'},
Copy link
Member Author

Choose a reason for hiding this comment

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

In the main page, a lot of headings are linked and jsdoc adds the link pieces into the heading but not GFM.

{find: '<h4 id="i-want-to-have-a-single-source-and-dont-care-about-live-adaptive-streaming">',
replace: '<h4 id="i-want-to-have-a-single-source-and-dont-care-about-liveadaptive-streaming">'},
{find: '<h2 id="api-docs-api">', replace: '<h2 id="api-docs">'},
{find: '<h2 id="guides-docs-guides">', replace: '<h2 id="guides">'}
Copy link
Member Author

Choose a reason for hiding this comment

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

various specific changes that are hard to target programmatically and are just changed directly.

Copy link
Member

Choose a reason for hiding this comment

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

I guess my only question with this section is whether it would make more sense to parse the HTML into a DOM and loop through all the heads, looking at their id attributes? It might be less error/typo-prone than this approach.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably best option would be to make a PR to jsdoc to expose the marker renderer so we can override the headings portion to our likings. For example, we're still missing a chain icon thing near each heading to click to be taken to that section.

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

One snark, one question. 😆

{find: /(\<h[1-6] id="(?:.*)?)video-js(.*)?"\>/g, replace: '$1videojs$2">'},
{find: /(\<h[1-6] id="(?:.*)?)don-t(.*)?"\>/g, replace: '$1dont$2">'},
{find: /(\<h[1-6] id="(?:.*)?)node-js(.*)?"\>/g, replace: '$1nodejs$2">'},
{find: /(\<h[1-6] id="(?:.*)?)vtt-js(.*)?"\>/g, replace: '$1vttjs$2">'},
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs changes, but I feel obliged to post this... https://stackoverflow.com/a/1732454

{find: '<h4 id="i-want-to-have-a-single-source-and-dont-care-about-live-adaptive-streaming">',
replace: '<h4 id="i-want-to-have-a-single-source-and-dont-care-about-liveadaptive-streaming">'},
{find: '<h2 id="api-docs-api">', replace: '<h2 id="api-docs">'},
{find: '<h2 id="guides-docs-guides">', replace: '<h2 id="guides">'}
Copy link
Member

Choose a reason for hiding this comment

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

I guess my only question with this section is whether it would make more sense to parse the HTML into a DOM and loop through all the heads, looking at their id attributes? It might be less error/typo-prone than this approach.

@gkatsev gkatsev merged commit 03fd402 into videojs:master Jul 14, 2017
@gkatsev gkatsev deleted the id-in-headings-jsdoc branch July 14, 2017 18:22
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

2 participants