Issue 6538, 6781 - Implement script parsing for snippets

lib/snippets.js

Index: lib/snippets.js
===================================================================
--- a/lib/snippets.js
+++ b/lib/snippets.js
@@ -70,8 +70,86 @@
       if (filter.isActiveOnDomain(domain))
         result.push(filter.script);
     }
     return result;
   }
 };
 
 exports.Snippets = Snippets;
+
+/**
+ * Parses a script and returns a list of all its commands and their arguments
+ * @param {string} script
+ * @return {Array.<string[]>}
+ */
+function parseScript(script)

[kzar, 2018/07/10 13:55:20]

I found this terminology confusing, "script" is the body of the snippet filter
which in turns calls a "snippet" script. How about `parseSnippetCalls`?

[Manish Jethani, 2018/07/11 19:04:41]

The argument to this function is the SnippetFilter.script property, I think we
agrees on that terminology. In that case parseScript sounds like an appropriate
name to me. What do you think?

[kzar, 2018/07/12 10:30:36]

Fair enough.

+{
+  const singleCharacterEscapes = new Map([

[kzar, 2018/07/10 13:55:20]

Might be better to declare this once at the top of the file, instead of every
time the function is called?

[Manish Jethani, 2018/07/11 19:04:41]

Done.

+    ["n", "\n"], ["r", "\r"], ["t", "\t"]
+  ]);
+
+  let tree = [];
+
+  let escape = false;
+  let literal = false;
+
+  let unicodeEscape = null;
+
+  let call = [];
+  let argument = "";
+
+  for (let character of [...script.trim(), ";"])

[kzar, 2018/07/10 13:55:20]

Why not just do `script = script.trim() + ";";` above and then `for (let
character of script)`?

[Manish Jethani, 2018/07/11 19:04:41]

Done.

+  {
+    if (unicodeEscape != null)
+    {
+      unicodeEscape += character;
+
+      if (unicodeEscape.length == 4)

[kzar, 2018/07/10 13:55:20]

What if unicodeEscape never reaches this length? I guess if the script ends with
"\u123" those last characters are lost?

[Manish Jethani, 2018/07/11 19:04:41]

Yes, they would be lost.

If it's not at the end of the script, then it'll eat up the 4 characters
following \u and then discard them because they don't parse as a number.

[kzar, 2018/07/12 10:30:36]

Acknowledged.

+      {
+        let codePoint = parseInt(unicodeEscape, 16);
+        if (!isNaN(codePoint))
+          argument += String.fromCodePoint(codePoint);
+
+        unicodeEscape = null;
+      }
+    }
+    else if (escape)
+    {
+      escape = false;
+
+      if (character == "u")
+        unicodeEscape = "";
+      else
+        argument += singleCharacterEscapes.get(character) || character;
+    }
+    else if (character == "\\")
+    {
+      escape = true;
+    }
+    else if (character == "'")
+    {
+      literal = !literal;
+    }
+    else if (literal || character != ";" && !/\s/u.test(character))

[kzar, 2018/07/10 13:55:20]

So the literal flag has no effect on escaped characters? For example, for the
literal string "\n" I'd have to use "\\n" or "'\\n'".

[Manish Jethani, 2018/07/11 19:04:41]

No, mainly because we need a way to escape the single quote within single
quotes. i.e. '\''. The single quotes in a way only group the characters within
as a single argument.

Literally what it's doing here is that if literal is true then it ignores
semicolons and whitespace and takes the character literally.

[kzar, 2018/07/12 10:30:36]

Acknowledged.

+    {
+      argument += character;
+    }
+    else
+    {
+      if (argument)
+      {
+        call.push(argument);
+        argument = "";
+      }
+
+      if (character == ";" && call.length > 0)
+      {
+        tree.push(call);
+        call = [];
+      }
+    }
+  }
+
+  return tree;
+}
+
+exports.parseScript = parseScript;