-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
feat(core/extensions): add extensions API #3320
base: main
Are you sure you want to change the base?
Conversation
Support two extension hooks: before-markdown and after-markdown. Support two extension utils: showError and showWarning.
I think this is still risky... I'd prefer people just user preProcess and postProcess. Hooking into anything makes things fragile, as we've seen. |
I understand the concern, but I think the previous hooks were fragile as we exposed them everywhere without much thought. If we limit what we expose, we can have a much more powerful and stable API. Also, when we think we need to remove some hook, we can deprecate them (as a breaking change), as we know what's exposed where. As an example, if you look at respec.org/docs/src.html#L29-L36 (and respec.org/docs/src.html#L98-L128 in particular), you'll see what hacks I had to make to overcome the limitations of preProcess and postProcess 😅 |
I see... what's a rough sketch of what that would look like with the new api? |
Looks cool. Which specs would use the markdown hooks? |
Haven't tested real specs that would like to use diff: docs/src.htmldiff --git a/static/docs/src.html b/static/docs/src.html
index 9c5375d..03418a3 100644
--- a/static/docs/src.html
+++ b/static/docs/src.html
@@ -34,6 +34,26 @@
postProcessEnhance,
cleanup,
],
+ extensions: [
+ {
+ name: "note-advisement",
+ on: "after-markdown",
+ run() {
+ for (const p of document.querySelectorAll("p")) {
+ if (p.firstElementChild?.localName === "strong") {
+ const text = p.firstElementChild.textContent.trimStart();
+ if (text.startsWith("Note:")) {
+ p.classList.add("note");
+ p.firstElementChild.remove();
+ } else if (text.startsWith("Warning:")) {
+ p.classList.add("advisement");
+ p.firstElementChild.remove();
+ }
+ }
+ }
+ },
+ },
+ ],
logos: [
{
src: "/respec-logo.png",
@@ -112,18 +132,6 @@
.replace(/\`\|(\w+)/g, `\`\\|${ZERO_WIDTH_SPACE}$1`)
.replace(/(\w+)\|\`/g, `$1${ZERO_WIDTH_SPACE}\\|\``);
- // Add .note and .advisement classes based on line prefix
- content = content
- .split("\n")
- .map(line => {
- if (/^.{0,5}\s*Warning:/.test(line))
- return `<div class="advisement">\n\n${line}</div>`;
- if (/^.{0,5}\s*Note:/.test(line))
- return `<div class="note">\n\n${line}</div>`;
- return line;
- })
- .join("\n");
-
return content;
} With a potential Similarly, if we could run Will test with some other real specs with potential hooks and update here. (PS: Missed notifications as I accidentally unsubscribed from this PR). Edits:
|
I still think this is going to be extremely footgunny.
Yeah, we should change the linters back into normal plugins as you suggested elsewhere. |
Closes https://github.com/w3c/respec/issues/1976
Support 4 extension hooks: before-all (preProcess), after-all (postProcess), before-markdown and after-markdown.
Support two extension utils: showError and showWarning.
TODO: