Skip to content

Commit

Permalink
tools: make sure doctool anchors respect includes
Browse files Browse the repository at this point in the history
Previously, output files which were created using includes (notably,
the single-page all.html) had basically broken internal links all
over the place because references like `errors.html#errors_class_error`
are being used, yet `id` attributes were generated that looked like
`all_class_error`.

This PR adds generation of comments from the include preprocessor
that indicate from which file the current markdown bits come and
lets the HTML output generation take advantage of that so that more
appropriate `id` attributes can be generated.

PR-URL: #6943
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: Daniel Wang <wangyang0123@gmail.com>
  • Loading branch information
addaleax authored and rvagg committed Jun 2, 2016
1 parent 2184e77 commit 108523e
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 18 deletions.
46 changes: 30 additions & 16 deletions test/doctool/test-doctool-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const assert = require('assert');
const fs = require('fs');
const path = require('path');

const processIncludes = require('../../tools/doc/preprocess.js');
const html = require('../../tools/doc/html.js');

// Test data is a list of objects with two properties.
Expand Down Expand Up @@ -53,30 +54,43 @@ const testData = [
'<p>Describe <code>Something</code> in more detail here. ' +
'</p>'
},
{
file: path.join(common.fixturesDir, 'doc_with_includes.md'),
html: '<!-- [start-include:doc_inc_1.md] -->' +
'<p>Look <a href="doc_inc_2.html#doc_inc_2_foobar">here</a>!</p>' +
'<!-- [end-include:doc_inc_1.md] -->' +
'<!-- [start-include:doc_inc_2.md] -->' +
'<h1>foobar<span><a class="mark" href="#doc_inc_2_foobar" ' +
'id="doc_inc_2_foobar">#</a></span></h1>' +
'<p>I exist and am being linked to.</p>' +
'<!-- [end-include:doc_inc_2.md] -->'
},
];

testData.forEach(function(item) {
// Normalize expected data by stripping whitespace
const expected = item.html.replace(/\s/g, '');

fs.readFile(item.file, 'utf8', common.mustCall(function(err, input) {
fs.readFile(item.file, 'utf8', common.mustCall((err, input) => {
assert.ifError(err);
html(
{
input: input,
filename: 'foo',
template: 'doc/template.html',
nodeVersion: process.version,
},
processIncludes(item.file, input, common.mustCall((err, preprocessed) => {
assert.ifError(err);

common.mustCall(function(err, output) {
assert.ifError(err);
html(
{
input: preprocessed,
filename: 'foo',
template: 'doc/template.html',
nodeVersion: process.version,
},
common.mustCall((err, output) => {
assert.ifError(err);

const actual = output.replace(/\s/g, '');
// Assert that the input stripped of all whitespace contains the
// expected list
assert.notEqual(actual.indexOf(expected), -1);
})
);
const actual = output.replace(/\s/g, '');
// Assert that the input stripped of all whitespace contains the
// expected list
assert.notEqual(actual.indexOf(expected), -1);
}));
}));
}));
});
3 changes: 3 additions & 0 deletions test/fixtures/doc_inc_1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Look [here][]!

[here]: doc_inc_2.html#doc_inc_2_foobar
3 changes: 3 additions & 0 deletions test/fixtures/doc_inc_2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# foobar

I exist and am being linked to.
2 changes: 2 additions & 0 deletions test/fixtures/doc_with_includes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@include doc_inc_1
@include doc_inc_2.md
17 changes: 16 additions & 1 deletion tools/doc/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,30 @@ function getSection(lexed) {
function buildToc(lexed, filename, cb) {
var toc = [];
var depth = 0;

const startIncludeRefRE = /^\s*<!-- \[start-include:(.+)\] -->\s*$/;
const endIncludeRefRE = /^\s*<!-- \[end-include:(.+)\] -->\s*$/;
const realFilenames = [filename];

lexed.forEach(function(tok) {
// Keep track of the current filename along @include directives.
if (tok.type === 'html') {
let match;
if ((match = tok.text.match(startIncludeRefRE)) !== null)
realFilenames.unshift(match[1]);
else if (tok.text.match(endIncludeRefRE))
realFilenames.shift();
}

if (tok.type !== 'heading') return;
if (tok.depth - depth > 1) {
return cb(new Error('Inappropriate heading level\n' +
JSON.stringify(tok)));
}

depth = tok.depth;
var id = getId(filename + '_' + tok.text.trim());
const realFilename = path.basename(realFilenames[0], '.md');
const id = getId(realFilename + '_' + tok.text.trim());
toc.push(new Array((depth - 1) * 2 + 1).join(' ') +
'* <a href="#' + id + '">' +
tok.text + '</a>');
Expand Down
6 changes: 5 additions & 1 deletion tools/doc/preprocess.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ function processIncludes(inputFile, input, cb) {
if (errState) return;
if (er) return cb(errState = er);
incCount--;
includeData[fname] = inc;

// Add comments to let the HTML generator know how the anchors for
// headings should look like.
includeData[fname] = `<!-- [start-include:${fname}] -->\n` +
inc + `\n<!-- [end-include:${fname}] -->\n`;
input = input.split(include + '\n').join(includeData[fname] + '\n');
if (incCount === 0) {
return cb(null, input);
Expand Down

0 comments on commit 108523e

Please sign in to comment.