Skip to content

Commit

Permalink
Relax duplicate filtering rules to only look at exported dfns (#1126)
Browse files Browse the repository at this point in the history
See w3c/webref#807 for context.

Reffy assumed that specs would define a term only once. The HTML spec has many
internal re-definitions of the same term. That's probably not fantastic, but not
fully wrong either, and references link to the right definition each time.

This adjusts the assumption to "specs export a term only once". Private
definitions are left intact (and these duplicates will re-appear in the
extracts).
  • Loading branch information
tidoust authored Nov 30, 2022
1 parent 49d8532 commit 7ceba09
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
11 changes: 6 additions & 5 deletions src/browserlib/extract-dfns.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,18 @@ function hasValidType(el) {
return isValid;
}

// Return true when definition is not already defined in the list,
// Return true when exported definition is not already defined in the list,
// Return false and issue a warning when it is already defined.
function isNotAlreadyDefined(dfn, idx, list) {
function isNotAlreadyExported(dfn, idx, list) {
const first = list.find(d => d === dfn ||
(d.type === dfn.type &&
(d.access === 'public' && dfn.access === 'public' &&
d.type === dfn.type &&
d.linkingText.length === dfn.linkingText.length &&
d.linkingText.every(lt => dfn.linkingText.find(t => t == lt)) &&
d.for.length === dfn.for.length &&
d.for.every(lt => dfn.for.find(t => t === lt))));
if (first !== dfn) {
console.warn('[reffy]', `Duplicate dfn found for "${dfn.linkingText[0]}", type="${dfn.type}", for="${dfn.for[0]}"`);
console.warn('[reffy]', `Duplicate dfn found for "${dfn.linkingText[0]}", type="${dfn.type}", for="${dfn.for[0]}", dupl=${dfn.href}, first=${first.href}`);
}
return first === dfn;
}
Expand Down Expand Up @@ -267,7 +268,7 @@ export default function (spec, idToHeading = {}) {
return !link || (node.textContent.trim() !== link.textContent.trim());
})
.map(node => definitionMapper(node, idToHeading, usesDfnDataModel))
.filter(isNotAlreadyDefined);
.filter(isNotAlreadyExported);
}

function preProcessEcmascript() {
Expand Down
6 changes: 3 additions & 3 deletions tests/extract-dfns.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ const tests = [
html: "<dfn id=foo data-dfn-type=invalidtype>Foo</dfn>",
changesToBaseDfn: []
},
{title: "ignores dfns already defined",
html: "<dfn id='foo' data-dfn-type='dfn'>Foo</dfn>. <dfn id='foo2'>Foo</dfn> is already defined.",
changesToBaseDfn: [{}]
{title: "ignores dfns already exported",
html: "<dfn id='foo' data-dfn-type='dfn' data-export>Foo</dfn>. <dfn id='foo2' data-export>Foo</dfn> is already exported.",
changesToBaseDfn: [{ access: "public" }]
},
{title: "automatically fixes internal slots dfns with an invalid 'idl' data-dfn-type",
html: "<dfn id=foo data-dfn-type=idl>Foo</dfn>",
Expand Down

0 comments on commit 7ceba09

Please sign in to comment.