Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow linking to lower level in delta specs. #1598

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bikeshed/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def __init__(self):
self.defaultHighlight = None
self.defaultBiblioDisplay = "index"
self.defaultRefStatus = None
self.deltaSpec = False
self.displayShortname = None
self.editors = []
self.editorTerm = {"singular": "Editor", "plural": "Editors"}
Expand Down Expand Up @@ -1320,6 +1321,7 @@ def parseLiteralList(key, val, lineNum): # pylint: disable=unused-argument
"Default Ref Status": Metadata(
"Default Ref Status", "defaultRefStatus", joinValue, parseRefStatus
),
"Delta Spec": Metadata("Delta Spec", "deltaSpec", joinValue, parseBoolean),
"ED": Metadata("ED", "ED", joinValue, parseLiteral),
"Editor": Metadata("Editor", "editors", joinList, parseEditor),
"Editor Term": Metadata("Editor Term", "editorTerm", joinValue, parseEditorTerm),
Expand Down
19 changes: 19 additions & 0 deletions bikeshed/refs/ReferenceManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class ReferenceManager:
"shortname",
"specLevel",
"spec",
"isDelta",
"testing",
]

Expand Down Expand Up @@ -93,6 +94,7 @@ def __init__(self, defaultStatus=None, fileRequester=None, testing=False):
self.shortname = None
self.specLevel = None
self.spec = None
self.isDelta = False

def initializeRefs(self, doc=None):
"""
Expand Down Expand Up @@ -236,6 +238,7 @@ def setSpecData(self, md):
self.shortname = md.shortname
self.specLevel = md.level
self.spec = md.vshortname
self.isDelta = md.deltaSpec

for term, defaults in md.linkDefaults.items():
for default in defaults:
Expand Down Expand Up @@ -506,6 +509,7 @@ def getRef(
export = True
else:
export = None

refs, failure = self.foreignRefs.queryRefs(
text=text,
linkType=linkType,
Expand All @@ -519,6 +523,21 @@ def getRef(
ignoreObsoletes=True,
)

if failure and self.isDelta:
Copy link
Collaborator

@tabatkins tabatkins Jun 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block doesn't do what you want. latestOnly just finds, for each dfn from each shortname, the highest-level version and then suppresses lower-level versions; if this is a delta spec that doesn't contain a term's dfn, it won't have a dfn in the results anyway so this won't have any effect.

(Also, it would only trigger if the first try completely failed; a confusable reference from another spec would prevent this from activating.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you instead want to alter is the ReferenceManager.removeSameSpecRefs() function; that's what unexports all dfns from specs with the same shortname.

(But make sure you still remove refs from the same or later level, so your local copy doesn't get confused with its version in the db, and doesn't link forward to a future version of itself either.)

refs, failure = self.foreignRefs.queryRefs(
text=text,
linkType=linkType,
spec=self.shortname,
status=status,
statusHint=statusHint,
linkFor=linkFor,
linkForHint=linkForHint,
explicitFor=explicitFor,
export=False,
ignoreObsoletes=True,
latestOnly=False,
)

if (
failure
and linkType in ("argument", "idl")
Expand Down