-
Notifications
You must be signed in to change notification settings - Fork 65
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
some tweaks to the biblio #405
Conversation
@@ -8,7 +8,7 @@ | |||
</ul></div><div id="spec-container"> | |||
<emu-clause id="sec-foo"> | |||
<h1><span class="secnum">1</span> Autolinking</h1> | |||
<p>Type, type, <emu-xref aoid="Type" id="_ref_0"><a href="https://tc39.es/ecma262/#sec-ecmascript-data-types-and-values">Type</a></emu-xref>(), type()</p> | |||
<p>Type, type, <emu-xref aoid="Type"><a href="https://tc39.es/ecma262/#sec-ecmascript-data-types-and-values">Type</a></emu-xref>(), type()</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was due to the "move logic for registering op biblio entries" commit.
Background: For xref
s to terms defined in the current document, we add an id
to the xref
so we can link to it from the References pane when hovering the definition of the term. That's what the id="_ref_0"
attributes are.
Prior to the referenced commit, we created the entry for an AO as part of adding the entry for a clause; that meant that when loading an external biblio containing an entry for a clause, we'd re-recreate the entry for its AO (thus shadowing the original one, from the external biblio). But all newly-created entries are considered to be in the current namespace, so the analysis thought the AO was defined in the current document, and hence xref
s to it should be given id
s.
The tweak in that commit means we don't re-create the biblio entry for the Type AO, and just use the original, external one. As a consequence, the analysis correctly recognizes that this xref is to an external term and so does not need to be given an id
. Hence the diff.
Also I think this fixes #252 (the "only include items in top-level namespace" commit). |
src/cli.ts
Outdated
@@ -107,6 +107,17 @@ const build = debounce(async function build() { | |||
} | |||
let warned = false; | |||
|
|||
for (let toResolve of args['load-biblio'] ?? []) { | |||
if (/^[./]/.test(toResolve)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (/^[./]/.test(toResolve)) { | |
if (/^\.{0,2}\//.test(toResolve)) { |
Maybe also pull the regexp out since it's used elsewhere.
breaking: rename --biblio to --write-biblio support loading biblio from specified path drop support for (very stale) ecma262 biblio allowlist rather than denylist for web-biblio in exported biblio, only include items in top-level namespace delete useless keys from exported biblio rename top-level namespace to external composition over inheritance more precise type for partial biblio entry move logic for registering op biblio entries as a consequence, fix a bug where adding an external biblio would re-create the "op" entries for clauses in the local biblio instead of leaving them external fix test hopefully
Some notable changes:
--no-ecma-262-biblio option
--biblio
with--write-biblio
, which is clearer--load-biblio
which takes either a path or an npm package to specify an external biblio, in preparation for Automatically update ecma262biblio.json #251 (may be specified multiple times)Also:
--write-biblio
version (again, only the things it uses)extends Array
; also renames the namespace holding entries from external biblios from "global" to "external".