★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21568Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

tools: build doc/api/all.html by combining generated HTML #21568

Closed
wants to merge 1 commit into from

Conversation

Reviewers

@jasnell jasnell

@vsemozhetbyt vsemozhetbyt

Assignees
No one assigned
Labels
build doc tools
Projects
None yet
Milestone
No milestone
5 participants
@rubys @Trott @vsemozhetbyt @jasnell @nodejs-github-bot
@rubys
Copy link
Member

rubys commented Jun 27, 2018

Combine the toc and api contents from the generated doc/api/*.html
files. This ensures that the single page version of the documentation
exactly matches the individual pages.

Fixes #20100

This pull request differs from #21544 in that it is implemented using regular expressions and has no dependencies. See #21544 (comment) for more context.

If this pull request is merged, the other will be deleted.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build doc tools labels Jun 27, 2018

@rubys rubys changed the title tools: build doc/api/all.html by combining generated HTML #21544 tools: build doc/api/all.html by combining generated HTML Jun 27, 2018

@vsemozhetbyt vsemozhetbyt referenced this pull request Jun 28, 2018

Closed

Produce HTML documentation using unified/remark/rehype #21490

3 of 3 tasks complete
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Jun 28, 2018

@TimothyGu @vsemozhetbyt

@rubys rubys referenced this pull request Jun 29, 2018

Closed

tools: build doc/api/all.html by combining generated HTML #21544

3 of 3 tasks complete
@jasnell

jasnell approved these changes Jun 29, 2018
View changes

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Jun 29, 2018

@nodejs/documentation It would be a good idea for someone to build all.html with this and compare it to what's in master. The diff should show some broken links being fixed but otherwise there should be no significant changes.

@rubys rubys force-pushed the rubys:build_api_all_from_generated_html2 branch from 338d466 to 0b9ff5f Jun 30, 2018

@rubys

This comment has been minimized.

Sign in to view
Copy link
Member Author

rubys commented Jun 30, 2018 •
edited

I've made a few fixes to reduce the differences, rebased to master, and here is a current set of differences: https://gist.github.com/rubys/bae70ed087efec625ac04823aaaf803e

Summary:

  • title is different; instead of picking the first title the new title is the part that is common to all titles. This could be changed, but I would still not recommend that it be the first title.
  • Some whitespace changes - nothing that would affect rendering
  • No include comments; something like that could be added if required, but likely would differ in that it would make more sense to reference the generated html files instead of the md source.
  • An odd change in api stability section; for reasons I don't understand the generation of this section added links previously. Links that are not present on the documentation page. Compare the current node documentation as published publicly: all.html vs documentation.html.
  • The previously generated all.html would sometimes link to definitions within the all.html page, and other times link to definitions on the separate page, and sometimes link to the wrong thing entirely. This has been changed to always link within the same page, and always to exactly what was linked to in the separate pages.
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jun 30, 2018

The same results here)

An odd change in api stability section...

This is OK. Due to this code:

node/tools/doc/html.js

Lines 209 to 216 in 1bf42f4

// Do not link to the section we are already in.
const noLinking = filename === 'documentation' &&
heading !== null && heading.text === 'Stability Index';
token.text = `<div class="api_stability api_stability_${number}">` +
(noLinking ? '' :
'<a href="/old?u=https%3A%2F%2Fgithub.com%2Fnodejs%2Fnode%2Fpull%2Fdocumentation.html%23documentation_stability_index&y=1999">') +
`${prefix} ${number}${noLinking ? '' : '</a>'}${explication}</div>`
.replace(/\n/g, ' ');

these links were skipped in documentation.html but wrongly reappeared in all.html. The new result is more consistent, I think.
@vsemozhetbyt

vsemozhetbyt approved these changes Jun 30, 2018
View changes

tools/doc/allhtml.js Outdated
let contents = '';
let apicontent = '';

// Identify files that should be skipped. As files are processed, they

This comment has been minimized.

Sign in to view
Copy link
@vsemozhetbyt

vsemozhetbyt Jun 30, 2018

Member

Nit: extra space before "As".
tools/doc/allhtml.js Outdated
.replace('api-section-_toc', 'api-section-all')
.replace('data-id="_toc"', 'data-id="all"');

// clean up the title.

This comment has been minimized.

Sign in to view
Copy link
@vsemozhetbyt

vsemozhetbyt Jun 30, 2018

Member

clean -> Clean ?
tools/doc/allhtml.js Outdated
'<ul>\n' + contents + '</ul>\n' +
all.slice(tocStart.index + tocStart[0].length);

// Replace apicontent with the contenated set of apicontents from each source.

This comment has been minimized.

Sign in to view
Copy link
@vsemozhetbyt

vsemozhetbyt Jun 30, 2018

Member

contenated -> concatenated?
@rubys
tools: build doc/api/all.html by combining generated HTML #21544 
Combine the toc and api contents from the generated doc/api/*.html
files. This ensures that the single page version of the documentation
exactly matches the individual pages.

Fixes #20100
Loading status checks…
6dac7db

@rubys rubys force-pushed the rubys:build_api_all_from_generated_html2 branch from 0b9ff5f to 6dac7db Jun 30, 2018

@rubys

This comment has been minimized.

Sign in to view
Copy link
Member Author

rubys commented Jun 30, 2018

Can somebody spot the CI error for me? I don't see it. I searched for 'error' and 'fail'.
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jun 30, 2018

You can also search for "not ok" in raw log: https://api.travis-ci.com/v3/job/132202841/log.txt
The test failure seems unrelated:
not ok 2146 es-module/test-esm-loader-invalid-format
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jun 30, 2018

Full CI: https://ci.nodejs.org/job/node-test-pull-request/15681/
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jun 30, 2018

Refs: #21605
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jun 30, 2018

#21605 is landed.
New full CI: https://ci.nodejs.org/job/node-test-pull-request/15684/
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Jun 30, 2018

And again... CI: https://ci.nodejs.org/job/node-test-pull-request/15695/
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jul 2, 2018

I shall land in some hours if there are no objections.
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jul 2, 2018

Landed in f85962f
Thank you!

@vsemozhetbyt vsemozhetbyt closed this Jul 2, 2018

vsemozhetbyt added a commit that referenced this pull request Jul 2, 2018

@rubys @vsemozhetbyt
tools: build all.html by combining generated HTML 
Combine the toc and api contents from the generated doc/api/*.html
files. This ensures that the single page version of the documentation
exactly matches the individual pages.

PR-URL: #21568
Fixes: #20100
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Loading status checks…
f85962f
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jul 2, 2018

Maybe we can do the same for the all.json so that we could completely rid of all.md. This can be done by JSON.parse() + combine new object + JSON.stringify() or just manually concatenate the new stringified serialization, not sure what would be easier or more effective. This may be semver-major as it may change the JSON structure users rely on programmatically.
@rubys

This comment has been minimized.

Sign in to view
Copy link
Member Author

rubys commented Jul 2, 2018

@vsemozhetbyt in the process, we could also get rid of preprocess.js and therefore processIncludes. Please open an issue and I'll work on a pull request.
@vsemozhetbyt

This comment has been minimized.

Sign in to view
Copy link
Member

vsemozhetbyt commented Jul 2, 2018

We usually do not require an issue for a PR. Feel free to pace as you feel comfortable)
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Jul 2, 2018 •
edited

Awesome to see "Contributor" on @rubys's comments etc. everywhere now. 🎉

targos added a commit that referenced this pull request Jul 3, 2018

@rubys @targos
tools: build all.html by combining generated HTML 
Combine the toc and api contents from the generated doc/api/*.html
files. This ensures that the single page version of the documentation
exactly matches the individual pages.

PR-URL: #21568
Fixes: #20100
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Verified
This commit was signed with a verified signature.
@targos targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
Loading status checks…
5d6d6fb

@targos targos referenced this pull request Jul 3, 2018

Merged

v10.6.0 proposal #21629

@rubys rubys referenced this pull request Jul 3, 2018

Closed

tools: build all.json by combining generated JSON #21637

3 of 3 tasks complete

@vsemozhetbyt vsemozhetbyt referenced this pull request Jul 9, 2018

Open

Creating a branch of the Docs using RunKit #21723

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.