Improve watching of tmlanguage and spec (#435)

* Fix null ref bug that broke spec generation
* Move processing in-proc to speed things up further
* Reduce polling interval to 200 ms
* Log diagnostics with appropriate source file
* Make output prettier
* Fix some breakage in launch.json after some things moved around

eng/scripts/helpers.js

@@ -94,29 +94,52 @@ export function clearScreen() {
   process.stdout.write("\x1bc");
 }
 
-export function runWatch(watch, dir, command, args, options) {
+export function runWatch(watch, dir, build, options) {
+  let lastStartTime;
   dir = resolve(dir);
-  handler();
 
-  watch.createMonitor(dir, (monitor) => {
+  // build once up-front.
+  runBuild();
+
+  watch.createMonitor(dir, { interval: 0.2, ...options }, (monitor) => {
+    let handler = function (file) {
+      if (lastStartTime && monitor?.files[file]?.mtime < lastStartTime) {
+        // File was changed before last build started so we can ignore it. This
+        // avoids running the build unnecessarily when a series of input files
+        // change at the same time.
+        return;
+      }
+      runBuild(file);
+    };
+
     monitor.on("created", handler);
     monitor.on("changed", handler);
     monitor.on("removed", handler);
   });
 
-  function handler(file) {
-    if (file && options.filter && !options.filter(file)) {
-      return;
+  function runBuild(file) {
+    runBuildAsync(file).catch((err) => {
+      console.error(err.stack);
+      process.exit(1);
+    });
+  }
+
+  async function runBuildAsync(file) {
+    lastStartTime = Date.now();
+    clearScreen();
+
+    if (file) {
+      logWithTime(`File change detected: ${file}. Running build.`);
+    } else {
+      logWithTime("Starting build in watch mode.");
     }
 
-    clearScreen();
-    logWithTime(`File changes detected in ${dir}. Running build.`);
-    const proc = run(command, args, { throwOnNonZeroExit: false, ...options });
-    console.log();
-    if (proc.status === 0) {
-      logWithTime("Build succeeded. Waiting for file changes...");
-    } else {
-      logWithTime(`Build failed with exit code ${proc.status}. Waiting for file changes...`);
+    try {
+      await build();
+      logWithTime("Build succeeded. Waiting for file changes.");
+    } catch (err) {
+      console.error(err.stack);
+      logWithTime(`Build failed. Waiting for file changes.`);
     }
   }
 }

packages/adl-language/scripts/watch-spec.js

@@ -1,8 +1,20 @@
 import watch from "watch";
+import ecmarkup from "ecmarkup";
+import { readFile } from "fs/promises";
 import { runWatch } from "../../../eng/scripts/helpers.js";
+import { resolve } from "path";
 
-runWatch(watch, "src", "node", [
-  "node_modules/ecmarkup/bin/ecmarkup.js",
-  "src/spec.emu.html",
-  "../../docs/spec.html",
-]);
+async function build() {
+  const infile = resolve("src/spec.emu.html");
+  const outfile = resolve("../../docs/spec.html");
+  const fetch = (path) => readFile(path, "utf-8");
+
+  try {
+    await ecmarkup.build(infile, fetch, { outfile });
+  } catch (err) {
+    console.log(`${infile}(1,1): error EMU0001: Error generating spec: ${err.message}`);
+    throw err;
+  }
+}
+
+runWatch(watch, "src", build);

packages/adl-language/src/spec.emu.html

@@ -18,7 +18,7 @@ InputElement :
     Token
     Trivia
 
-Token:
+Token :
     Keyword
     Identifier
     NumericLiteral

packages/adl-vscode/package.json

@@ -29,7 +29,7 @@
     "watch": "tsc -p . --watch",
     "watch-tmlanguage": "node scripts/watch-tmlanguage.js",
     "dogfood": "node scripts/dogfood.js",
-    "generate-tmlanguage": "node dist-dev/tmlanguage.js",
+    "generate-tmlanguage": "node scripts/generate-tmlanguage.js",
     "generate-third-party-notices": "node scripts/generate-third-party-notices.js",
     "rollup": "rollup --config --failAfterWarnings 2>&1",
     "package-vsix": "vsce package --yarn"

packages/adl-vscode/scripts/generate-tmlanguage.js

@@ -0,0 +1,12 @@
+import { createRequire } from "module";
+import { resolve } from "path";
+
+const require = createRequire(import.meta.url);
+const script = resolve("dist-dev/tmlanguage.js");
+
+require(script)
+  .main()
+  .catch((err) => {
+    console.error(err.stack);
+    process.exit(1);
+  });

packages/adl-vscode/scripts/watch-tmlanguage.js

@@ -1,7 +1,26 @@
 import watch from "watch";
 import { runWatch } from "../../../eng/scripts/helpers.js";
-import { basename } from "path";
+import { resolve } from "path";
+import { createRequire } from "module";
 
-runWatch(watch, "dist-dev", "node", ["dist-dev/tmlanguage.js"], {
-  filter: (file) => basename(file) === "tmlanguage.js",
+const require = createRequire(import.meta.url);
+const script = resolve("dist-dev/tmlanguage.js");
+
+async function regenerate() {
+  // For perf, we don't want to shell out to a new process every build and we
+  // particularly want to avoid reinitialzing onigasm, which is relatively slow.
+  // So we purge the script from the require cache and re-run it with changes
+  // in-proc.
+  delete require.cache[script];
+  await require(script).main();
+}
+
+runWatch(watch, "dist-dev", regenerate, {
+  // This filter doesn't do as much as one might hope because tsc writes out all
+  // the files on recompilation. So tmlanguage.js changes when other .ts files
+  // in adl-vscode change but tmlanguage.ts has not changed. We could check the
+  // tmlanguage.ts timestamp to fix it, but it didn't seem worth the complexity.
+  // We can't just watch tmlanguage.ts because we need to wait for tsc to
+  // compile it.
+  filter: (file) => file === script,
 });

packages/adl-vscode/src/tmlanguage.ts

@@ -3,6 +3,7 @@
 
 import * as tm from "@azure-tools/tmlanguage-generator";
 import fs from "fs/promises";
+import { resolve } from "path";
 
 type IncludeRule = tm.IncludeRule<ADLScope>;
 type BeginEndRule = tm.BeginEndRule<ADLScope>;
@@ -319,12 +320,9 @@ const grammar: Grammar = {
   patterns: [statement],
 };
 
-async function main() {
-  const plist = await tm.emitPList(grammar);
+export async function main() {
+  const plist = await tm.emitPList(grammar, {
+    errorSourceFilePath: resolve("./src/tmlanguage.ts"),
+  });
   await fs.writeFile("./dist/adl.tmLanguage", plist);
 }
-
-main().catch((err) => {
-  console.log(err.stack);
-  process.exit(1);
-});

packages/tmlanguage-generator/src/tmlanguage-generator.ts

@@ -74,22 +74,26 @@ async function initialize() {
   }
 }
 
+export interface EmitOptions {
+  errorSourceFilePath?: string;
+}
+
 /**
  * Emit the given grammar to JSON.
  */
-export async function emitJSON(grammar: Grammar): Promise<string> {
+export async function emitJSON(grammar: Grammar, options: EmitOptions = {}): Promise<string> {
   await initialize();
   const indent = 2;
-  const processed = await processGrammar(grammar);
+  const processed = await processGrammar(grammar, options);
   return JSON.stringify(processed, undefined, indent);
 }
 
 /**
  * Emit the given grammar to PList XML
  */
-export async function emitPList(grammar: Grammar): Promise<string> {
+export async function emitPList(grammar: Grammar, options: EmitOptions = {}): Promise<string> {
   await initialize();
-  const processed = await processGrammar(grammar);
+  const processed = await processGrammar(grammar, options);
   return plist.build(processed);
 }
 
@@ -97,22 +101,22 @@ export async function emitPList(grammar: Grammar): Promise<string> {
  * Convert the grammar from our more convenient representation to the
  * tmlanguage schema. Perform some validation in the process.
  */
-async function processGrammar(grammar: Grammar): Promise<any> {
+async function processGrammar(grammar: Grammar, options: EmitOptions): Promise<any> {
   await initialize();
 
   // key is rule.key, value is [unprocessed rule, processed rule]. unprocessed
   // rule is used for its identity to check for duplicates and deal with cycles.
   const repository = new Map<string, [Rule, any]>();
-  const output = processNode(grammar);
+  const output = processNode(grammar, options);
   output.repository = processRepository();
   return output;
 
-  function processNode(node: any): any {
+  function processNode(node: any, options: EmitOptions): any {
     if (typeof node !== "object") {
       return node;
     }
     if (Array.isArray(node)) {
-      return node.map(processNode);
+      return node.map((n) => processNode(n, options));
     }
     const output: any = {};
     for (const key in node) {
@@ -130,28 +134,28 @@ async function processGrammar(grammar: Grammar): Promise<any> {
         case "begin":
         case "end":
         case "match":
-          validateRegexp(value, node, key);
+          validateRegexp(value, node, key, options);
           output[key] = value;
           break;
         case "patterns":
-          output[key] = processPatterns(value);
+          output[key] = processPatterns(value, options);
           break;
         default:
-          output[key] = processNode(value);
+          output[key] = processNode(value, options);
           break;
       }
     }
     return output;
   }
 
-  function processPatterns(rules: Rule[]) {
+  function processPatterns(rules: Rule[], options: EmitOptions) {
     for (const rule of rules) {
       if (!repository.has(rule.key)) {
         // put placeholder first to prevent cycles
         const entry: [Rule, any] = [rule, undefined];
         repository.set(rule.key, entry);
         // fill placeholder with processed node.
-        entry[1] = processNode(rule);
+        entry[1] = processNode(rule, options);
       } else if (repository.get(rule.key)![0] !== rule) {
         throw new Error("Duplicate key: " + rule.key);
       }
@@ -168,18 +172,19 @@ async function processGrammar(grammar: Grammar): Promise<any> {
     return output;
   }
 
-  function validateRegexp(regexp: string, node: any, prop: string) {
+  function validateRegexp(regexp: string, node: any, prop: string, options: EmitOptions) {
     try {
       new OnigRegExp(regexp).testSync("");
     } catch (err) {
-      if (/^[0-9,]+/.test(err.message)) {
+      if (/^[0-9,]+$/.test(err.message)) {
         // Work around for https://github.com/NeekSandhu/onigasm/issues/26
         const array = new Uint8Array(err.message.split(",").map((s: string) => Number(s)));
         const buffer = Buffer.from(array);
         err = new Error(buffer.toString("utf-8"));
       }
+      const sourceFile = options.errorSourceFilePath ?? "unknown_file";
       //prettier-ignore
-      console.error(`unknown_location(1,1): error TM0001: Bad regex encountered while generating .tmLanguage: ${JSON.stringify({[prop]: regexp})}`);
+      console.error(`${sourceFile}(1,1): error TM0001: Bad regex: ${JSON.stringify({[prop]: regexp})}: ${err.message}`);
       console.error(node);
       throw err;
     }