From 504bfa6e603edc4f5ec432eb8317f23a566c68c6 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Sun, 18 Jun 2023 15:51:00 -0400 Subject: [PATCH] Fix / with structured headers (#533) --- src/Clause.ts | 44 +++++++++++-------- src/Production.ts | 18 +++----- src/clauseNums.ts | 26 ++++++----- src/header-parser.ts | 2 +- src/utils.ts | 31 ++++++++----- test/baselines.js | 32 ++++++++++++-- .../generated-reference/clauses.html | 4 +- .../generated-reference/duplicate-ids.html | 5 +++ .../baselines/generated-reference/figure.html | 6 +-- .../generated-reference/namespaces.html | 7 +++ .../structured-headers.html | 7 +-- test/baselines/sources/clauses.html | 2 +- test/baselines/sources/duplicate-ids.html | 5 +++ test/baselines/sources/figure.html | 6 +-- test/baselines/sources/namespaces.html | 7 +++ .../baselines/sources/structured-headers.html | 3 +- 16 files changed, 136 insertions(+), 69 deletions(-) diff --git a/src/Clause.ts b/src/Clause.ts index 4c01c218..48c7c1e1 100644 --- a/src/Clause.ts +++ b/src/Clause.ts @@ -13,7 +13,7 @@ import { parseH1, ParsedHeader, } from './header-parser'; -import { offsetToLineAndColumn } from './utils'; +import { offsetToLineAndColumn, traverseWhile } from './utils'; const aoidTypes = [ 'abstract operation', @@ -32,7 +32,11 @@ export const SPECIAL_KINDS_MAP = new Map([ export const SPECIAL_KINDS = [...SPECIAL_KINDS_MAP.keys()]; export function extractStructuredHeader(header: Element): Element | null { - const dl = header.nextElementSibling; + const dl = traverseWhile( + header.nextElementSibling, + 'nextElementSibling', + el => el.nodeName === 'DEL' + ); if (dl == null || dl.tagName !== 'DL' || !dl.classList.contains('header')) { return null; } @@ -95,39 +99,43 @@ export default class Clause extends Builder { } this.signature = null; - let header = this.node.firstElementChild; - while (header != null && header.tagName === 'SPAN' && header.children.length === 0) { - // skip oldids - header = header.nextElementSibling; - } - if (header == null) { + const header = traverseWhile( + this.node.firstElementChild, + 'nextElementSibling', + // skip and oldids + el => el.nodeName === 'DEL' || (el.nodeName === 'SPAN' && el.children.length === 0) + ); + let headerH1 = traverseWhile(header, 'firstElementChild', el => el.nodeName === 'INS', { + once: true, + }); + if (headerH1 == null) { this.spec.warn({ type: 'node', ruleId: 'missing-header', message: `could not locate header element`, node: this.node, }); - header = null; - } else if (header.tagName !== 'H1') { + headerH1 = null; + } else if (headerH1.tagName !== 'H1') { this.spec.warn({ type: 'node', ruleId: 'missing-header', - message: `could not locate header element; found <${header.tagName.toLowerCase()}> before any

`, - node: header, + message: `could not locate header element; found <${header!.tagName.toLowerCase()}> before any

`, + node: header!, }); - header = null; + headerH1 = null; } else { - this.buildStructuredHeader(header); + this.buildStructuredHeader(headerH1, header!); } - this.header = header; - if (header == null) { + this.header = headerH1; + if (headerH1 == null) { this.title = 'UNKNOWN'; this.titleHTML = 'UNKNOWN'; } } - buildStructuredHeader(header: Element) { - const dl = extractStructuredHeader(header); + buildStructuredHeader(header: Element, headerSurrogate: Element = header) { + const dl = extractStructuredHeader(headerSurrogate); if (dl === null) { return; } diff --git a/src/Production.ts b/src/Production.ts index 6219bb85..f1a432a6 100644 --- a/src/Production.ts +++ b/src/Production.ts @@ -52,18 +52,14 @@ export default class Production extends Builder { if (node.hasAttribute('primary')) { primary = true; } else { - let parent = node.parentElement; - if (parent != null) { + const parent = utils.traverseWhile( + node.parentElement, + 'parentElement', // highlighted nodes still count as primary unless they are being deleted (i.e. in a tag) - while (parent.tagName === 'INS' || parent.tagName === 'MARK') { - parent = parent.parentElement; - if (parent == null) { - break; - } - } - if (parent != null && parent.tagName === 'EMU-GRAMMAR') { - primary = parent.hasAttribute('primary') || parent.getAttribute('type') === 'definition'; - } + el => el.nodeName === 'INS' || el.nodeName === 'MARK' + ); + if (parent != null && parent.tagName === 'EMU-GRAMMAR') { + primary = parent.hasAttribute('primary') || parent.getAttribute('type') === 'definition'; } } this.primary = primary; diff --git a/src/clauseNums.ts b/src/clauseNums.ts index ce74228a..c289d8af 100644 --- a/src/clauseNums.ts +++ b/src/clauseNums.ts @@ -108,17 +108,21 @@ export default function iterator(spec: Spec): ClauseNumberIterator { message: 'multi-step explicit clause numbers should not be mixed with single-step clause numbers in the same parent clause', }); - } else if ( - nums.some((n, i) => n < ids[level][i]) || - nums.every((n, i) => n === ids[level][i]) - ) { - spec.warn({ - type: 'attr-value', - node, - attr: 'number', - ruleId: 'invalid-clause-number', - message: 'clause numbers should be strictly increasing', - }); + } else { + // Make sure that `nums` is strictly greater than `ids[level]` (i.e., + // that their items are not identical and that the item in `nums` is + // strictly greater than the value in `ids[level]` at the first + // index where they differ). + const i = nums.findIndex((num, i) => num !== ids[level][i]); + if (i < 0 || !(nums[i] > ids[level][i])) { + spec.warn({ + type: 'attr-value', + node, + attr: 'number', + ruleId: 'invalid-clause-number', + message: 'clause numbers should be strictly increasing', + }); + } } } return nums; diff --git a/src/header-parser.ts b/src/header-parser.ts index 14449de8..519187cf 100644 --- a/src/header-parser.ts +++ b/src/header-parser.ts @@ -237,7 +237,7 @@ export function parseH1(headerText: string): ParsedHeaderOrFailure { if (match) { offset += match[0].length; returnOffset = offset; - ({ match, text } = eat(text, /^(.*)(?!<\/(ins|del|mark)>)/i)); + ({ match, text } = eat(text, /^(.*?)(?=<\/(ins|del|mark)>|$)/im)); if (match) { returnType = match[1].trim(); if (returnType === '') { diff --git a/src/utils.ts b/src/utils.ts index 8818c16e..fe12d8d6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -117,6 +117,21 @@ export function replaceTextNode(node: Node, frag: DocumentFragment) { return newXrefNodes; } +/*@internal*/ +export function traverseWhile

>( + node: T | null, + relationship: P, + predicate: (node: T) => boolean, + options?: { once?: boolean } +): T | null { + const once = options?.once ?? false; + while (node != null && predicate(node)) { + node = node[relationship]; + if (once) break; + } + return node; +} + /*@internal*/ export function logVerbose(str: string) { const dateString = new Date().toISOString(); @@ -132,20 +147,12 @@ export function logWarning(str: string) { const CLAUSE_LIKE = ['EMU-ANNEX', 'EMU-CLAUSE', 'EMU-INTRO', 'EMU-NOTE', 'BODY']; /*@internal*/ export function shouldInline(node: Node) { - let parent = node.parentNode; + const surrogateParentTags = ['EMU-GRAMMAR', 'EMU-IMPORT', 'INS', 'DEL']; + const parent = traverseWhile(node.parentNode, 'parentNode', node => + surrogateParentTags.includes(node?.nodeName ?? '') + ); if (!parent) return false; - while ( - parent && - parent.parentNode && - (parent.nodeName === 'EMU-GRAMMAR' || - parent.nodeName === 'EMU-IMPORT' || - parent.nodeName === 'INS' || - parent.nodeName === 'DEL') - ) { - parent = parent.parentNode; - } - const clauseLikeParent = CLAUSE_LIKE.includes(parent.nodeName) || CLAUSE_LIKE.includes((parent as Element).getAttribute('data-simulate-tagname')?.toUpperCase()!); diff --git a/test/baselines.js b/test/baselines.js index c31e92f7..c76e8878 100644 --- a/test/baselines.js +++ b/test/baselines.js @@ -35,7 +35,8 @@ describe('baselines', () => { let optionsSets = [{ lintSpec: false }]; for (let file of files) { const reference = REFERENCE_DIR + file; - it(SOURCES_DIR + file, async () => { + const sourcePath = SOURCES_DIR + file; + it(sourcePath, async () => { let expectedFiles = new Map(); (function walk(f) { @@ -51,7 +52,7 @@ describe('baselines', () => { } })(reference); - let spec = await build(SOURCES_DIR + file, {}); + let spec = await build(sourcePath, {}); let actualFiles = handleSingleFileOutput(spec.generatedFiles); @@ -81,14 +82,39 @@ describe('baselines', () => { if (rebaseline) { return; } + + let contents = fs.readFileSync(sourcePath, 'utf8'); + let expectedWarnings = [...contents.matchAll(//g)].map(m => + JSON.parse(m[1]) + ); + let warningProps = new Set(expectedWarnings.flatMap(obj => Object.keys(obj))); + function pickFromWarning(warning) { + if (warningProps.size === 0) { + // No warnings are expected, so "pick" the entire object. + return warning; + } + let picks = {}; + for (let [key, value] of Object.entries(warning)) { + if (warningProps.has(key)) picks[key] = value; + } + return picks; + } + for (let options of optionsSets) { - let spec = await build(SOURCES_DIR + file, options); + let warnings = []; + let warn = warning => warnings.push(warning); + let spec = await build(sourcePath, { ...options, warn }); let actualFiles = handleSingleFileOutput(spec.generatedFiles); assert.deepStrictEqual( actualFiles, expectedFiles, `output differed when using option ${JSON.stringify(options)}` ); + assert.deepStrictEqual( + warnings.map(warning => pickFromWarning(warning)), + expectedWarnings, + `warnings differed when using option ${JSON.stringify(options)}` + ); } }); } diff --git a/test/baselines/generated-reference/clauses.html b/test/baselines/generated-reference/clauses.html index 3b804a5a..5a791dfd 100644 --- a/test/baselines/generated-reference/clauses.html +++ b/test/baselines/generated-reference/clauses.html @@ -6,7 +6,7 @@

  • Jump to search box/

    • 1 A

    +

    1.1 Sub A

    @@ -16,6 +17,7 @@

    1.1 Sub A

         +

    2 Section A: Extras

    Table 1 (Informative): A Table Of Stuff
    @@ -25,14 +27,17 @@

    2 Section A: Extras

         +

    3 Section A: Extras

    +
    Table 2 (Informative): A Table Of Stuff
    Column 1Column 2
    ValueValue 2
     +
    Example (Informative): An example
    Multiple examples are numbered similar to notes
    diff --git a/test/baselines/generated-reference/figure.html b/test/baselines/generated-reference/figure.html index f73ce94f..2be4e686 100644 --- a/test/baselines/generated-reference/figure.html +++ b/test/baselines/generated-reference/figure.html @@ -38,12 +38,12 @@ -
    Figure 3 (Informative): This is the caption
    +
    Figure 3 (Informative): This is the caption
    this is a figure!
    -
    Table 3: This is a table
    +
    Table 3: This is a table
    @@ -56,7 +56,7 @@
    -
    Table 4 (Informative): This is a second table
    +
    Table 4 (Informative): This is a second table
    diff --git a/test/baselines/generated-reference/namespaces.html b/test/baselines/generated-reference/namespaces.html index ca1fbf1d..cdd67cb2 100644 --- a/test/baselines/generated-reference/namespaces.html +++ b/test/baselines/generated-reference/namespaces.html @@ -44,6 +44,8 @@

    1.1 Clause 1.1

    + +

    1.1.1 SomeAlg

     @@ -63,10 +65,14 @@

    A Annex

    + +

    A.1 SomeAlg

     + +

    A.2 Annex 1.2

    SomeAlg should link to #annex12. Foo should link to the production in #annex1.

    @@ -78,6 +84,7 @@

    A.2 Annex 1.2

    B Annex 2

    +

    B.1 SomeAlg

     diff --git a/test/baselines/generated-reference/structured-headers.html b/test/baselines/generated-reference/structured-headers.html index 205f9bbd..e995a2cc 100644 --- a/test/baselines/generated-reference/structured-headers.html +++ b/test/baselines/generated-reference/structured-headers.html @@ -17,6 +17,7 @@

    1 ExampleAO ( param [ , param2 ] )

    1. Algorithm steps go here.
     +

    2 ExampleAO ( )

    The abstract operation ExampleAO takes no arguments. It redefines an existing algorithm. It performs the following steps when called:

    @@ -87,9 +88,9 @@

    6.2 IsThat

     - -

    7 ExampleAO3 ( param )

    -

    The abstract operation ExampleAO3 takes argument param (an integer) and returns the return type. It performs the following steps when called:

    + +

    7 ExampleAO5 ( param )

    +

    The abstract operation ExampleAO5 takes argument param (an integer) and returns the return type. It performs the following steps when called:

    1. Algorithm steps go here.
         diff --git a/test/baselines/sources/clauses.html b/test/baselines/sources/clauses.html index b13d4753..96594410 100644 --- a/test/baselines/sources/clauses.html +++ b/test/baselines/sources/clauses.html @@ -48,7 +48,7 @@

    Nested clause after explicit multi-step number

     - +

    Increasing multi-step explicit numbers

         diff --git a/test/baselines/sources/duplicate-ids.html b/test/baselines/sources/duplicate-ids.html index 48ed7c7a..3848687f 100644 --- a/test/baselines/sources/duplicate-ids.html +++ b/test/baselines/sources/duplicate-ids.html @@ -5,6 +5,7 @@

    A

    +

    Sub A

    @@ -13,6 +14,7 @@

    Sub A

         +

    Section A: Extras

    @@ -22,14 +24,17 @@

    Section A: Extras

    Column 1Column 2
    +

    Section A: Extras

    +
    Column 1Column 2
    ValueValue 2
     + Multiple examples are numbered similar to notes diff --git a/test/baselines/sources/figure.html b/test/baselines/sources/figure.html index f739db79..fa0139e7 100644 --- a/test/baselines/sources/figure.html +++ b/test/baselines/sources/figure.html @@ -35,12 +35,12 @@ - + This is the caption this is a figure! - + This is a table @@ -53,7 +53,7 @@
    - + This is a second table diff --git a/test/baselines/sources/namespaces.html b/test/baselines/sources/namespaces.html index 62342d10..3b5d37e9 100644 --- a/test/baselines/sources/namespaces.html +++ b/test/baselines/sources/namespaces.html @@ -27,6 +27,8 @@

    Clause 1.1

    `bar` MainProd `baz` + +

    SomeAlg

     @@ -42,10 +44,14 @@

    Annex

    Foo :: `bar` MainProd `baz` + +

    SomeAlg

     + +

    Annex 1.2

    SomeAlg should link to #annex12. |Foo| should link to the production in #annex1.

    @@ -59,6 +65,7 @@

    Annex 1.2

    Annex 2

    +

    SomeAlg

     diff --git a/test/baselines/sources/structured-headers.html b/test/baselines/sources/structured-headers.html index d890415b..d8e5bc15 100644 --- a/test/baselines/sources/structured-headers.html +++ b/test/baselines/sources/structured-headers.html @@ -23,6 +23,7 @@

    +

    ExampleAO ( )

    @@ -126,7 +127,7 @@

    - ExampleAO3 ( + ExampleAO5 ( param : an integer, ): the return type

    Column 1Column 2