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

fix(core/xref): improve errors, show for context #3656

Merged
merged 5 commits into from
Jun 29, 2021
Merged
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
40 changes: 23 additions & 17 deletions src/core/xref.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import { cacheXrefData, resolveXrefCache } from "./xref-db.js";
import {
createResourceHint,
docLink,
joinAnd,
joinOr,
nonNormativeSelector,
norm as normalize,
showError,
Expand Down Expand Up @@ -147,9 +150,7 @@ function normalizeConfig(xref) {
return config;

function invalidProfileError(profile) {
const supportedProfiles = Object.keys(profiles)
.map(p => `"${p}"`)
.join(", ");
const supportedProfiles = joinOr(Object.keys(profiles), s => `"${s}"`);
const msg =
`Invalid profile "${profile}" in \`respecConfig.xref\`. ` +
`Please use one of the supported profiles: ${supportedProfiles}.`;
Expand Down Expand Up @@ -435,8 +436,8 @@ function addToReferences(elem, cite, normative, term, conf) {
return;
}

const msg = `Normative reference to "${term}" found but term is defined informatively in "${cite}"`;
const title = "Error: Normative reference to informative term";
const msg = `Normative reference to "${term}" found but term is defined "informatively" in "${cite}".`;
const title = "Normative reference to non-normative term.";
showWarning(msg, name, { title, elements: [elem] });
}

Expand All @@ -448,32 +449,37 @@ function showErrors({ ambiguous, notFound }) {
if (query.for) url.searchParams.set("for", query.for);
url.searchParams.set("types", query.types.join(","));
if (specs.length) url.searchParams.set("specs", specs.join(","));
return url;
return url.href;
};

const howToFix = howToCiteURL =>
"[Learn more about this error](https://respec.org/docs/#error-term-not-found)" +
` or see [how to cite to resolve the error](${howToCiteURL}).`;
const howToFix = (howToCiteURL, originalTerm) => {
return docLink`
[See search matches for "${originalTerm}"](${howToCiteURL}) or
${"[Learn about this error|#error-term-not-found]"}.`;
};

for (const { query, elems } of notFound.values()) {
const specs = query.specs ? [...new Set(query.specs.flat())].sort() : [];
const originalTerm = getTermFromElement(elems[0]);
const formUrl = getPrefilledFormURL(originalTerm, query);
const specsString = specs.map(spec => `\`${spec}\``).join(", ");
const hint = howToFix(formUrl);
const msg = `Couldn't match "**${originalTerm}**" to anything in the document or in any other document cited in this specification: ${specsString}.`;
const title = "Error: No matching dfn found.";
const specsString = joinAnd(specs, s => `**[${s}]**`);
const hint = howToFix(formUrl, originalTerm);
const forParent = query.for ? `, for **"${query.for}"**, ` : "";
const msg = `Couldn't find "**${originalTerm}**"${forParent} in this document or other cited documents: ${specsString}.`;
const title = "No matching definition found.";
showError(msg, name, { title, elements: elems, hint });
}

for (const { query, elems, results } of ambiguous.values()) {
const specs = [...new Set(results.map(entry => entry.shortname))].sort();
const specsString = specs.map(s => `**${s}**`).join(", ");
const specsString = joinAnd(specs, s => `**[${s}]**`);
const originalTerm = getTermFromElement(elems[0]);
const formUrl = getPrefilledFormURL(originalTerm, query, specs);
const hint = howToFix(formUrl);
const msg = `The term "**${originalTerm}**" is defined in ${specsString} in multiple ways, so it's ambiguous.`;
const title = "Error: Linking an ambiguous dfn.";
const forParent = query.for ? `, for **"${query.for}"**, ` : "";
const moreInfo = howToFix(formUrl, originalTerm);
const hint = docLink`To fix, use the ${"[data-cite]"} attribute to pick the one you mean from the appropriate specification. ${moreInfo}.`;
const msg = `The term "**${originalTerm}**"${forParent} is ambiguous because it's defined in ${specsString}.`;
const title = "Definition is ambiguous.";
showError(msg, name, { title, elements: elems, hint });
}
}
Expand Down
12 changes: 5 additions & 7 deletions tests/spec/core/xref-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe("Core — xref", () => {
const link = doc.getElementById("external-link");
expect(link.classList).toContain("respec-offending-element");
expect(link.getAttribute("href")).toBeFalsy();
expect(link.title).toBe("Error: No matching dfn found.");
expect(link.title).toBe("No matching definition found.");
});

it("uses data-cite to disambiguate", async () => {
Expand Down Expand Up @@ -150,7 +150,7 @@ describe("Core — xref", () => {

const link = doc.querySelector("#test a");
expect(link.classList).toContain("respec-offending-element");
expect(link.title).toBe("Error: Linking an ambiguous dfn.");
expect(link.title).toBe("Definition is ambiguous.");
});

it("uses data-cite to disambiguate - 2", async () => {
Expand Down Expand Up @@ -186,7 +186,7 @@ describe("Core — xref", () => {
const five = doc.getElementById("five");
expect(five.href).toBe("");
expect(five.classList).toContain("respec-offending-element");
expect(five.title).toBe("Error: No matching dfn found.");
expect(five.title).toBe("No matching definition found.");
});

it("uses data-cite fallbacks", async () => {
Expand Down Expand Up @@ -226,7 +226,7 @@ describe("Core — xref", () => {
const link3 = doc.getElementById("link3");
expect(link3.href).toEqual("https://fetch.spec.whatwg.org/");
expect(link3.classList).toContain("respec-offending-element");
expect(link3.title).toEqual("Error: No matching dfn found.");
expect(link3.title).toEqual("No matching definition found.");

const linkLocal0 = doc.getElementById("link-local-0");
expect(linkLocal0.getAttribute("href")).toEqual("#dfn-local");
Expand Down Expand Up @@ -551,9 +551,7 @@ describe("Core — xref", () => {
"https://www.w3.org/TR/css-values-4/#bearing-angle"
);
expect(badLink.classList).toContain("respec-offending-element");
expect(badLink.title).toBe(
"Error: Normative reference to informative term"
);
expect(badLink.title).toBe("Normative reference to non-normative term.");

const normRefs = [...doc.querySelectorAll("#normative-references dt")];
expect(normRefs).toHaveSize(1); // excludes `css-values` of `#invalid`
Expand Down