respec icon indicating copy to clipboard operation
respec copied to clipboard

feat(core/extensions): add extensions API

Open sidvishnoi opened this issue 3 years ago • 6 comments

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:

  • [ ] investigate more hooks to start with
  • [ ] investigate more utils to start with
  • [ ] write tests

sidvishnoi avatar Feb 19 '21 21:02 sidvishnoi

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.

marcoscaceres avatar Feb 22 '21 14:02 marcoscaceres

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 :sweat_smile:

sidvishnoi avatar Feb 23 '21 09:02 sidvishnoi

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?

marcoscaceres avatar Feb 24 '21 08:02 marcoscaceres

Looks cool. Which specs would use the markdown hooks?

saschanaz avatar Mar 04 '21 06:03 saschanaz

Haven't tested real specs that would like to use after-markdown hook, but respec.org/docs/src.html#L115-L125 could use it like this:

diff: docs/src.html
diff --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 after-inline-processing hook (which runs before linker), docs/src.html#L99-L113 will simplify a lot.

Similarly, if we could run fixLinks(), say, on: "after-linker", we would get access to local-refs-exist linker in a better way. Currently linters run after doc.respec.ready, which means we can miss linter warnings in respec2html (to workaround this, we wait for 1 second after respec.ready which is problematic in itself).

Will test with some other real specs with potential hooks and update here.

(PS: Missed notifications as I accidentally unsubscribed from this PR).

Edits:

  • Many ARIA/A11Y, JSON-LD and other specs could benefit from after-linker event. They currently use pubsubhub.end["core/link-to-dfn"]. Maybe we can expose after-local-linker and after-external-linker (need to be careful of course).
  • A lot of specs could use utils.showError() and utils.showWarning(). Though, they could as well be added to preProcess,postProcess, and isn't an endorsement for new extension API.

sidvishnoi avatar Mar 21 '21 14:03 sidvishnoi

I still think this is going to be extremely footgunny.

Currently linters run after doc.respec.ready, which means we can miss linter warnings in respec2html (to workaround this, we wait for 1 second after respec.ready which is problematic in itself).

Yeah, we should change the linters back into normal plugins as you suggested elsewhere.

marcoscaceres avatar Mar 23 '21 07:03 marcoscaceres