Skip to content

Commit

Permalink
Prevent resolveReferences.js from pruning internal references we use
Browse files Browse the repository at this point in the history
The script logic was returning early for all internal references
which were not also directly referenced by the main content (for
instance "class" and "text node" from Core AAM).

The script already keeps track of unused (by the main content) in
the termNames array. Therefore, we can check that array to see if
it contains the parent glossary term. If so, then return early like
before. Otherwise, let the script proceed.

Addresses github issue #20.
  • Loading branch information
joanmarie committed Jan 16, 2019
1 parent bb88d41 commit 15714ed
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions script/resolveReferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ function updateReferences(base) {
}

// We should be able to remove terms that are not actually
// referenced from the common definitions
// referenced from the common definitions. This array is
// indexed with the element ids of the dfn tags to be pruned.
var termNames = [] ;

function restrictReferences(utils, content) {
Expand All @@ -224,7 +225,8 @@ function restrictReferences(utils, content) {
// New logic: If the reference is within a 'dl' element of
// class 'termlist', and if the target of that reference is
// also within a 'dl' element of class 'termlist', then
// consider it an internal reference and ignore it.
// consider it an internal reference and ignore it -- assuming
// it is not part of another included term.

require(["core/pubsubhub"], function(respecEvents) {
"use strict";
Expand All @@ -236,8 +238,14 @@ require(["core/pubsubhub"], function(respecEvents) {
var t = $item.attr('href');
if ( $item.closest('dl.termlist').length ) {
if ( $(t).closest('dl.termlist').length ) {
// do nothing
return;
// Figure out the id of the glossary term which holds this
// internal reference and see if it will be pruned (i.e.
// is in the termNames array). If it is, we can ignore
// this particular internal reference.

This comment has been minimized.

Copy link
@jnurthen

jnurthen Jan 17, 2019

Member

Wouldn't this logic prune a term if it had a double reference.
i.e. we have term apple
the definition for apple references banana
the definition for banana references cherry
the definition for cherry should not be pruned

unless i am misunderstanding this there is no recursion here so we risk this scenario (unless we agree we should better define our terms so this doesn't happen)

This comment has been minimized.

Copy link
@joanmarie

joanmarie Jan 17, 2019

Author Contributor

Yeah, there's no recursion. So I believe you're correct. Bah. (And good catch.) I'll do a new version tomorrow that handles that scenario.

Thanks for the review!

var dfn = $item.closest('dd').prev().find('dfn');
var parentTermId = dfn.makeID('dfn', dfn.getDfnTitles[0]);
if (termNames[parentTermId])
return;
}
}
var r = t.replace(/^#/,"") ;
Expand Down

0 comments on commit 15714ed

Please sign in to comment.