Re: [heycam/webidl] Check grammar is LL(1) as part of Travis builds (#458)

domenic approved this pull request.

JS style nits (feel free to ignore them) and a somewhat substantial deploy script issue, but LGTM otherwise. Thanks for cleaning up my mistakes with the extended attributes business. LL(1) seems like a pain.

> @@ -0,0 +1,48 @@
+const Grammar = require("syntax-cli").Grammar;

Nit: `"use strict";`

> @@ -0,0 +1,48 @@
+const Grammar = require("syntax-cli").Grammar;
+const LLParsingTable = require("syntax-cli").LLParsingTable;
+const jsdom = require("jsdom");
+
+function getRulesFromDOM(window) {
+    let rules = window.document.querySelectorAll("pre.grammar[id]");
+    return [].map.call(rules, pre => pre.textContent);
+}
+
+function processRules(rules) {
+    const REGEXP = /(\s*:\n\s*)|\b(integer|float|identifier|string|whitespace|comment|other)\b|(\s*\n\s*)|(ε)/g;
+    return rules.map(rule => {
+        return  rule.trim().replace(REGEXP, m => {

Nit: two spaces after `return` seems silly.

> +            if (/^(integer|float|identifier|string|whitespace|comment|other)$/.test(m)) {
+              return m.toUpperCase();
+            }
+            if (/:\n/.test(m)) { return "\n  : "; }
+            if (/\n/.test(m)) { return "\n  | "; }
+            if (/ε/.test(m)) { return "/* epsilon */"; }
+        }) + "\n  ;";
+    });
+}
+
+function toBNF(rules) {
+    return "\n%%\n\n" + processRules(rules).join("\n");
+}
+
+let path = process.argv[2];
+let html = require("fs").readFileSync(path, "utf8");

Nit: seems nicer to put all `require()`s at the top

> @@ -13,7 +13,8 @@ if [[ "$TRAVIS_PULL_REQUEST" != "false" || "$TRAVIS_BRANCH" != "$SOURCE_BRANCH"
   echo "Skipping deploy; just doing a build."
   mkdir out
   doCompile
-  exit 0
+  node ./check-grammar.js ./out/index.html
+  exit $?

Why only check on pull requests?

I'd just put this inside `doCompile`. No need for `exit $?` then since we have done `set -e` above.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/458#pullrequestreview-68468125

Received on Wednesday, 11 October 2017 01:00:27 UTC