From 66d4d12d0e1c631fe5d9f781d9c5fa5b3bf6091d Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Tue, 25 Nov 2025 17:39:32 -0700 Subject: [PATCH] resolves #121 log warning if link to external resource cannot be made because site URL is not set --- CHANGELOG.adoc | 5 ++ .../assembler/lib/produce-assembly-file.js | 65 +++++++++++++------ .../assembler/lib/produce-assembly-files.js | 1 + .../assembler/test/assemble-content-test.js | 1 + .../test/produce-assembly-files-test.js | 16 ++++- .../expects/index.adoc | 2 +- .../rewrite-page-links/expects/index.adoc | 2 +- packages/test-harness/lib/index.js | 3 +- packages/test-harness/lib/load-scenario.js | 2 + 9 files changed, 72 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 140f781..830adba 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -10,6 +10,11 @@ For a detailed view of what's changed, refer to the {url-repo}/commits[commit hi * Create relative link even when site URL is not set (#135) +=== Changed + +* Log warning if link to external resource cannot be made because site URL is not set (#121) +* Rewrite xref as link with unresolved role when link to external resource cannot be made (#121) + == 1.0.0-beta.14 (2025-11-24) === Changed diff --git a/packages/assembler/lib/produce-assembly-file.js b/packages/assembler/lib/produce-assembly-file.js index 87329cf..5f64a01 100644 --- a/packages/assembler/lib/produce-assembly-file.js +++ b/packages/assembler/lib/produce-assembly-file.js @@ -121,6 +121,7 @@ function mergeAsciiDoc ( outDirname, siteRoot, xmlIds, + logger, } = assemblyModel // FIXME: ideally, resource ID would be stored in navigation so we can look up the page more efficiently const page = urlType === 'internal' && !unresolved ? pagesInOutline.get(url) : undefined @@ -142,6 +143,16 @@ function mergeAsciiDoc ( Object.assign({}, page, { contents: trimAsciiDoc(contents), mediaType }) ) const doc = loadAsciiDoc(pageAsAsciiDoc, contentCatalog, asciidocConfig) + let asciidoctorLogger = doc.getLogger() + if (logger) { + asciidoctorLogger = Object.assign(asciidoctorLogger.$dup(), { + delegate: { + warn () { + return logger.warn.apply(logger, arguments) + }, + }, + }) + } if (doc.hasAttribute('assembly-navtitle')) { navtitleAsciiDoc = doc.getAttribute('assembly-navtitle') navtitlePlain = sanitize((navtitle = doc.$apply_reftext_subs(navtitleAsciiDoc))) @@ -374,7 +385,7 @@ function mergeAsciiDoc ( } if (~line.indexOf('xref:')) { // Q: should we allow : as first character of target? - line = line.replace(/(? { + line = line.replace(/(? { let fragment, resource, resourceRef const hashIdx = target.indexOf('#') if (~hashIdx) { @@ -389,21 +400,23 @@ function mergeAsciiDoc ( // Q: should we validate the internal ID here? if (!resourceRef) return `xref:${idPrefix}${fragment}[${text}]` const resourceId = parseResourceRef(resourceRef, page.src, 'page', contentCatalog) - if (resourceId.family !== 'page') { - if ((siteRoot || linkRefStyle === 'relative') && (resource = contentCatalog.getById(resourceId))?.pub) { - text ||= resource.asciidoc?.xreftext || target - return `${resolveLinkTarget(resource, siteRoot, pubRoot, linkRefStyle)}${fragment && '#' + fragment}[${text}]` - } - // TODO: handle unresolved resource better - return m - } - if (!(resource = pagesInOutline.get(createResourceKey(resourceId)))) { - if ((siteRoot || linkRefStyle === 'relative') && (resource = contentCatalog.getById(resourceId))?.pub) { + if (resourceId.family !== 'page' || !(resource = pagesInOutline.get(createResourceKey(resourceId)))) { + if ((resource = contentCatalog.getById(resourceId))?.pub) { text ||= resource.asciidoc?.xreftext || target - return `${resolveLinkTarget(resource, siteRoot, pubRoot, linkRefStyle)}${fragment && '#' + fragment}[${text}]` + if (siteRoot || linkRefStyle === 'relative') { + return `${resolveLinkTarget(resource, siteRoot, pubRoot, linkRefStyle)}${fragment && '#' + fragment}[${text}]` + } + asciidoctorLogger.warn( + doc.createLogMessage( + `Cannot create external ${resourceId.family} reference in assembly because site URL is unknown: ${target}`, + { source_location: { file: relative, lineno: idx + 1 } } + ) + ) } - // TODO: handle unresolved page better - return m + const linkAttrlist = text + ? (~text.indexOf(',') ? `"${text}"` : text) + ',role=unresolved' + : 'role=unresolved' + return `link:${target}[${linkAttrlist}]` } if (fragment === resource.asciidoc.id) fragment = '' if ( @@ -441,15 +454,25 @@ function mergeAsciiDoc ( } if (~line.indexOf('image:') && !line.startsWith('image::')) { line = line.replace(/(? { - if (isResourceSpec(target)) { - const image = contentCatalog.resolveResource(target, page.src, 'image', ['image']) - // TODO: handle (or report) unresolved image better - if (image?.out && (filetype === 'html' ? siteRoot || linkRefStyle === 'relative' : true)) { + let image + if ( + isResourceSpec(target) && + (image = contentCatalog.resolveResource(target, page.src, 'image', ['image']))?.out + ) { + if (filetype !== 'html') { + pagesInOutline.assembled.assets.add(image) + return `image:${resolveEmbedTarget(image, outDirname, embedRefStyle, true)}[${attrlist}]` + } + if (siteRoot || linkRefStyle === 'relative') { pagesInOutline.assembled.assets.add(image) - return filetype === 'html' - ? `image:${resolveLinkTarget(image, siteRoot, pubRoot, linkRefStyle)}[${attrlist}]` - : `image:${resolveEmbedTarget(image, outDirname, embedRefStyle, true)}[${attrlist}]` + return `image:${resolveLinkTarget(image, siteRoot, pubRoot, linkRefStyle)}[${attrlist}]` } + asciidoctorLogger.warn( + doc.createLogMessage( + `Cannot create external image reference in assembly because site URL is unknown: ${target}`, + { source_location: { file: relative, lineno: idx + 1 } } + ) + ) } return m }) diff --git a/packages/assembler/lib/produce-assembly-files.js b/packages/assembler/lib/produce-assembly-files.js index 3a26fa6..07d1452 100644 --- a/packages/assembler/lib/produce-assembly-files.js +++ b/packages/assembler/lib/produce-assembly-files.js @@ -21,6 +21,7 @@ function produceAssemblyFiles (loadAsciiDoc, contentCatalog, assemblerConfig, re embedReferenceStyle: assemblyConfig.embedReferenceStyle, linkReferenceStyle: assemblyConfig.linkReferenceStyle, dropExplicitXrefText: assemblyConfig.dropExplicitXrefText, + logger: assemblyConfig.logger, // for tests }) const assemblerAsciiDocAttributes = Object.assign({}, assemblerAsciiDocConfig.attributes) const { revdate, 'source-highlighter': sourceHighlighter } = assemblerAsciiDocAttributes diff --git a/packages/assembler/test/assemble-content-test.js b/packages/assembler/test/assemble-content-test.js index 75fde39..0fe562b 100644 --- a/packages/assembler/test/assemble-content-test.js +++ b/packages/assembler/test/assemble-content-test.js @@ -142,6 +142,7 @@ describe('assembleContent()', () => { return doc } const converter = { convert, backend: 'pdf', extname: '.pdf', mediaType: 'application/pdf' } + delete configSource.assembly.logger const generatorContext = createGeneratorContext() generatorContext.getLogger = () => logger await assembleContent.call(generatorContext, playbook, contentCatalog, converter, { configSource }) diff --git a/packages/assembler/test/produce-assembly-files-test.js b/packages/assembler/test/produce-assembly-files-test.js index a41a991..adedb5a 100644 --- a/packages/assembler/test/produce-assembly-files-test.js +++ b/packages/assembler/test/produce-assembly-files-test.js @@ -1,6 +1,6 @@ 'use strict' -const { assert, assertx, describe, it, loadScenario, runScenario } = require('test-harness') +const { assert, assertx, configureLogger, describe, it, loadScenario, runScenario } = require('test-harness') const produceAssemblyFiles = require('@antora/assembler/produce-assembly-files') describe('produceAssemblyFiles()', () => { @@ -195,7 +195,21 @@ describe('produceAssemblyFiles()', () => { }) it('should rewrite page links when site-url not set', async () => { + const messages = [] + const expected = { + level: 'warn', + file: { + path: 'modules/ROOT/pages/the-page.adoc', + line: 5, + }, + msg: 'Cannot create external page reference in assembly because site URL is unknown: other-component::index.adoc', + } + configureLogger({ level: 'all', destination: { write: (message) => messages.push(message) } }) await runScenario('rewrite-page-links-without-site-url', __dirname) + assert.equal(messages.length, 1) + const actual = JSON.parse(messages[0]) + assertx.partialDeepEqual(actual, expected) + assertx.doesNotHaveProperty(actual, 'name') }) it('should rewrite relative page links', async () => { diff --git a/packages/assembler/test/scenarios/rewrite-page-links-without-site-url/expects/index.adoc b/packages/assembler/test/scenarios/rewrite-page-links-without-site-url/expects/index.adoc index 55c364a..c8789f6 100644 --- a/packages/assembler/test/scenarios/rewrite-page-links-without-site-url/expects/index.adoc +++ b/packages/assembler/test/scenarios/rewrite-page-links-without-site-url/expects/index.adoc @@ -21,4 +21,4 @@ You are on xref:the-page[this page]. -Go back to the site to see xref:other-component::index.adoc[]. +Go back to the site to see link:other-component::index.adoc[Another Page,role=unresolved]. diff --git a/packages/assembler/test/scenarios/rewrite-page-links/expects/index.adoc b/packages/assembler/test/scenarios/rewrite-page-links/expects/index.adoc index cad0ebb..5fe1010 100644 --- a/packages/assembler/test/scenarios/rewrite-page-links/expects/index.adoc +++ b/packages/assembler/test/scenarios/rewrite-page-links/expects/index.adoc @@ -46,7 +46,7 @@ Or you can learn all about xref:\stem:index[STEM]. Go back to the site to see https://preview-docs.example.org/other-component/index.html[Another Page] and https://docs.example.org/imported-component/index.html[Imported Page]. -You won't be able to see xref:_unpublished.adoc[] since it is not published. +You won't be able to see link:_unpublished.adoc[role=unresolved] since it is not published. You have reached the end of xref:the-page[xrefstyle=full]. diff --git a/packages/test-harness/lib/index.js b/packages/test-harness/lib/index.js index c3941a8..f5a0d26 100644 --- a/packages/test-harness/lib/index.js +++ b/packages/test-harness/lib/index.js @@ -12,7 +12,7 @@ const heredoc = require('./heredoc') const loadScenario = require('./load-scenario') const runScenario = require('./run-scenario') -beforeEach(() => configureLogger({ level: 'all' })) // eslint-disable-line no-undef +beforeEach(() => configureLogger({ level: 'all' })) module.exports = { after, @@ -21,6 +21,7 @@ module.exports = { assertx, before, beforeEach, + configureLogger, createFile, createGeneratorContext, describe, diff --git a/packages/test-harness/lib/load-scenario.js b/packages/test-harness/lib/load-scenario.js index 8b8c2f7..a6191ff 100644 --- a/packages/test-harness/lib/load-scenario.js +++ b/packages/test-harness/lib/load-scenario.js @@ -1,5 +1,6 @@ 'use strict' +const { getLogger } = require('@antora/logger') const { loadAsciiDoc, resolveAsciiDocConfig } = require('@antora/asciidoc-loader') const loadAssemblerConfig = require('@antora/assembler/load-config') const parseResourceRef = require('@antora/assembler/parse-resource-ref') @@ -234,6 +235,7 @@ async function loadScenario (name, dirname) { } } const assemblerConfig = await loadAssemblerConfig(playbook, data.assembler) + assemblerConfig.assembly.logger = getLogger() const nav = data.navigation if (Array.isArray(nav) && !nav.length) { componentVersion.navigation = [] // emulate nav file with no contents -- GitLab