From 108523e06e42040ce1f2ab10d3e89092d5311661 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 24 May 2016 00:06:09 +0200 Subject: [PATCH] tools: make sure doctool anchors respect includes 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: https://github.com/nodejs/node/pull/6943 Reviewed-By: Robert Jefe Lindstaedt Reviewed-By: Daniel Wang --- test/doctool/test-doctool-html.js | 46 +++++++++++++++++++----------- test/fixtures/doc_inc_1.md | 3 ++ test/fixtures/doc_inc_2.md | 3 ++ test/fixtures/doc_with_includes.md | 2 ++ tools/doc/html.js | 17 ++++++++++- tools/doc/preprocess.js | 6 +++- 6 files changed, 59 insertions(+), 18 deletions(-) create mode 100644 test/fixtures/doc_inc_1.md create mode 100644 test/fixtures/doc_inc_2.md create mode 100644 test/fixtures/doc_with_includes.md diff --git a/test/doctool/test-doctool-html.js b/test/doctool/test-doctool-html.js index 8e16403901a00a..91b9e0de6d3777 100644 --- a/test/doctool/test-doctool-html.js +++ b/test/doctool/test-doctool-html.js @@ -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. @@ -53,30 +54,43 @@ const testData = [ '

Describe Something in more detail here. ' + '

' }, + { + file: path.join(common.fixturesDir, 'doc_with_includes.md'), + html: '' + + '

Look here!

' + + '' + + '' + + '

foobar#

' + + '

I exist and am being linked to.

' + + '' + }, ]; 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); + })); + })); })); }); diff --git a/test/fixtures/doc_inc_1.md b/test/fixtures/doc_inc_1.md new file mode 100644 index 00000000000000..92858d0200c237 --- /dev/null +++ b/test/fixtures/doc_inc_1.md @@ -0,0 +1,3 @@ +Look [here][]! + +[here]: doc_inc_2.html#doc_inc_2_foobar diff --git a/test/fixtures/doc_inc_2.md b/test/fixtures/doc_inc_2.md new file mode 100644 index 00000000000000..17d0b86a0cc51f --- /dev/null +++ b/test/fixtures/doc_inc_2.md @@ -0,0 +1,3 @@ +# foobar + +I exist and am being linked to. diff --git a/test/fixtures/doc_with_includes.md b/test/fixtures/doc_with_includes.md new file mode 100644 index 00000000000000..901bf0f1b0bf3b --- /dev/null +++ b/test/fixtures/doc_with_includes.md @@ -0,0 +1,2 @@ +@include doc_inc_1 +@include doc_inc_2.md diff --git a/tools/doc/html.js b/tools/doc/html.js index de63aefa43b510..769d601e26c800 100644 --- a/tools/doc/html.js +++ b/tools/doc/html.js @@ -283,7 +283,21 @@ function getSection(lexed) { function buildToc(lexed, filename, cb) { var toc = []; var depth = 0; + + const startIncludeRefRE = /^\s*\s*$/; + const endIncludeRefRE = /^\s*\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' + @@ -291,7 +305,8 @@ function buildToc(lexed, filename, cb) { } 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(' ') + '* ' + tok.text + ''); diff --git a/tools/doc/preprocess.js b/tools/doc/preprocess.js index 295737a2a53aee..55d90996f71c13 100644 --- a/tools/doc/preprocess.js +++ b/tools/doc/preprocess.js @@ -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] = `\n` + + inc + `\n\n`; input = input.split(include + '\n').join(includeData[fname] + '\n'); if (incCount === 0) { return cb(null, input);