From 9ef6e23088ed4a310d1eee31eec821fd059624d2 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 8e16403901..91b9e0de6d 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 0000000000..92858d0200 --- /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 0000000000..17d0b86a0c --- /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 0000000000..901bf0f1b0 --- /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 de63aefa43..769d601e26 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 295737a2a5..55d90996f7 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);