Remove QueryList in favour of QueryBinary, adding a root Query node t…

…o permit empty queries But typeahead is still broken
guardian · Nov 8, 2024 · 492a63d · 492a63d
1 parent 8c280e8
commit 492a63d
Show file tree

Hide file tree

Showing 12 changed files with 150 additions and 156 deletions.
diff --git a/README.md b/README.md
@@ -58,19 +58,15 @@ sausages
 Grammar:
 
 ```
-query_list                  -> query* EOF
-query                       -> query_binary | query_field | query_output_modifier
-query_binary                -> query_content ('AND' | 'OR' query_content)*
+query                       -> query_binary?
+query_binary                -> query_content (('AND' | 'OR')? query_content)*
 query_content               -> query_group | query_str | query_quoted_str
 query_group                 -> '(' query_binary* ')'
 query_quoted_str            -> '"' string '"'
 query_str                   -> /\w/
 query_field                 -> '+' query_field_key ':'? query_field_value? // Permit incomplete meta queries for typeahead
 query_field_key             -> 'tag' | 'section' | ...etc
 query_field_value           -> /\w/
-query_output_modifier       -> '@' query_output_modifier_key ':'? query_output_modifier_value
-query_output_modifier_key   -> 'show-fields' ...etc
-query_output_modifier_value -> /\w/
 ```
 
 How do we disambiguate search params from strings in the tokeniser?
@@ -94,8 +90,8 @@ Typeahead will require parsing AST nodes, not just tokens, as typeahead for quer
 
 Typeahead happens on server. Rationale: typeahead must hit CAPI anyhow, so it's dependent on some server somewhere, and making it an LS feature keeps the client simpler.
 
-What do we serve for typeahead? 
-1. Provide values for every incomplete query_field node for every query. Client then keeps those values for display when selection is in right place. 
+What do we serve for typeahead?
+1. Provide values for every incomplete query_field node for every query. Client then keeps those values for display when selection is in right place.
 2. Provide typehead as combination of position and query to server. Store less data upfront, but must query every time position changes.
 
 Option 1. preferable to avoid high request volumes, keep typeahead in sync with query, and keep latency low (chance that typeahead result will be cached, when for example clicking into existing incomplete query_field)

diff --git a/prosemirror-client/src/cqlInput/editor/debug.ts b/prosemirror-client/src/cqlInput/editor/debug.ts
@@ -2,11 +2,11 @@ import { Node } from "prosemirror-model";
 import { Mapping } from "prosemirror-transform";
 import { Token } from "../../lang/token";
 import {
+  Query,
   QueryBinary,
   QueryContent,
   QueryField,
   QueryGroup,
-  QueryList,
   QueryStr,
 } from "../../lang/ast";
 
@@ -187,25 +187,17 @@ export const getDebugMappingHTML = (
   //  return `<div class="CqlDebug__mapping">${queryDiagram}${nodeDiagram}</div>`;
 };
 
-export const getDebugASTHTML = (query: QueryList) => {
+export const getDebugASTHTML = (query: Query) => {
   return `<div class="tree--container">
-    ${getQueryListHTML(query)}
+    <ul class="tree">
+      <li>
+        <span>${getNodeHTML(query)}</span>
+        ${query.content ? getBinaryHTML(query.content) : ""}
+      </li>
+    </ul>
   </div>`;
 };
 
-const getQueryListHTML = (list: QueryList) => {
-  return `<ul class="tree">
-    <li>
-      ${getNodeHTML(list)}
-      <ul>
-      ${list.content
-        .map((binary) => `<li>${getBinaryHTML(binary)}</li>`)
-        .join("")}
-      </ul>
-    </li>
-  </ul>`;
-};
-
 const getContentHTML = (query: QueryContent) => {
   const html = (() => {
     switch (query.content.type) {
@@ -230,14 +222,21 @@ const getContentHTML = (query: QueryContent) => {
 };
 
 const getBinaryHTML = (query: QueryBinary): string => {
+  const maybeRight = query.right?.[1];
+
+  const binaryContent = maybeRight
+    ? `
+     <ul>
+        <li>${getContentHTML(query.left)}</li>
+        <li>${getBinaryHTML(maybeRight)}</li>
+      </ul>`
+    : getContentHTML(query.left);
+
   return `
-    <ul>
+    <ul class="tree">
       <li>
         <span>${getNodeHTML(query)}</span>
-        <ul>
-          <li>${getContentHTML(query.left)}</li>
-          ${query.right?.[1] ? `<li>${getBinaryHTML(query.right[1])}</li>` : ""}
-        </ul>
+        ${binaryContent}
       </li>
     </ul>
   `;
@@ -270,7 +269,7 @@ const getGroupHTML = (group: QueryGroup) => {
     <ul>
       <li>
         ${getNodeHTML(group)}
-        ${getQueryListHTML(group.content)}
+        ${getBinaryHTML(group.content)}
       </li>
     </ul>
   `;

diff --git a/prosemirror-client/src/cqlInput/editor/plugin.spec.ts b/prosemirror-client/src/cqlInput/editor/plugin.spec.ts
@@ -129,7 +129,7 @@ const selectPopoverOption = async (
   await editor.press("Enter");
 };
 
-describe("editor", () => {
+describe("plugin", () => {
   beforeEach(() => {
     document.body.innerHTML = "";
   });

diff --git a/prosemirror-client/src/lang/Cql.spec.ts b/prosemirror-client/src/lang/Cql.spec.ts
@@ -3,7 +3,7 @@ import { TestTypeaheadHelpers } from "./typeaheadHelpersTest";
 import { Typeahead } from "./typeahead";
 import { Cql } from "./Cql";
 
-describe("a program", () => {
+describe("a query", () => {
   const typeaheadHelpers = new TestTypeaheadHelpers();
   const typeahead = new Typeahead(typeaheadHelpers.fieldResolvers);
 

diff --git a/prosemirror-client/src/lang/Cql.ts b/prosemirror-client/src/lang/Cql.ts
@@ -1,5 +1,5 @@
 import { either, Result } from "../utils/result";
-import { QueryList } from "./ast";
+import { Query } from "./ast";
 import { queryStrFromQueryList } from "./capiQueryString";
 import { Parser } from "./parser";
 import { Scanner } from "./scanner";
@@ -9,8 +9,8 @@ import { TypeaheadSuggestion } from "./types";
 
 export type CqlResult = {
   tokens: Token[];
-  ast?: QueryList;
-  suggestions?: TypeaheadSuggestion[];
+  ast?: Query;
+  suggestions: TypeaheadSuggestion[];
   queryResult?: string;
   error?: Error;
 };
@@ -37,6 +37,7 @@ export class Cql {
           new CqlResultEnvelope({
             tokens,
             error,
+            suggestions: [],
           })
         ),
       async (queryArr) => {

diff --git a/prosemirror-client/src/lang/ast.ts b/prosemirror-client/src/lang/ast.ts
@@ -1,12 +1,12 @@
 import { Token } from "./token";
 
-export type QueryList = {
-  type: "QueryList";
-  content: QueryBinary[];
+export type Query = {
+  type: "Query";
+  content?: QueryBinary;
 };
 
-export const createQueryList = (content: QueryList["content"]): QueryList => ({
-  type: "QueryList",
+export const createQuery = (content?: QueryBinary): Query => ({
+  type: "Query",
   content,
 });
 
@@ -37,7 +37,7 @@ export const createQueryContent = (
   content,
 });
 
-export type QueryGroup = { type: "QueryGroup"; content: QueryList };
+export type QueryGroup = { type: "QueryGroup"; content: QueryBinary };
 
 export const createQueryGroup = (
   content: QueryGroup["content"]

diff --git a/prosemirror-client/src/lang/capiQueryString.ts b/prosemirror-client/src/lang/capiQueryString.ts
@@ -1,66 +1,58 @@
 import { err, ok, Result } from "../utils/result";
-import { QueryBinary, QueryContent, QueryList } from "./ast";
-import { getQueryFieldsFromQueryList } from "./util";
+import { Query, QueryBinary, QueryContent } from "./ast";
+import { getQueryFieldsFromQueryBinary } from "./util";
 
 class CapiQueryStringError extends Error {
   public constructor(message: string) {
     super(message);
   }
 }
 
-export const queryStrFromQueryList = (
-  list: QueryList
-): Result<Error, string> => {
-  const searchStrs = strFromList(list);
+export const queryStrFromQueryList = (query: Query): Result<Error, string> => {
+  const { content } = query;
+  if (!content) {
+    return ok("");
+  }
+
+  const searchStrs = strFromBinary(content);
 
   try {
-    const otherQueries = getQueryFieldsFromQueryList(list).flatMap((expr) => {
-      switch (expr.type) {
-        case "QueryField": {
-          if (expr.value) {
-            return [`${expr.key.literal ?? ""}=${expr.value.literal ?? ""}`];
-          } else {
-            throw new CapiQueryStringError(
-              `The field '+$${expr.key}' needs a value after it (e.g. +${expr.key}:tone/news)`
-            );
+    const otherQueries = getQueryFieldsFromQueryBinary(content).flatMap(
+      (expr) => {
+        switch (expr.type) {
+          case "QueryField": {
+            if (expr.value) {
+              return [`${expr.key.literal ?? ""}=${expr.value.literal ?? ""}`];
+            } else {
+              throw new CapiQueryStringError(
+                `The field '+$${expr.key}' needs a value after it (e.g. +${expr.key}:tone/news)`
+              );
+            }
+          }
+          default: {
+            return [];
           }
-        }
-        default: {
-          return [];
         }
       }
-    });
-
-    const maybeSearchStr = searchStrs.join(" ").trim();
+    );
 
-    const searchStr = maybeSearchStr.length
-      ? `q=${encodeURI(maybeSearchStr)}`
+    const queryStr = searchStrs.length
+      ? `q=${encodeURI(searchStrs.trim())}`
       : "";
 
-    return ok([searchStr, otherQueries].filter(Boolean).flat().join("&"));
+    return ok([queryStr, otherQueries].filter(Boolean).flat().join("&"));
   } catch (e) {
     return err(e as Error);
   }
 };
 
-const strFromList = (list: QueryList): string[] => {
-  return list.content.flatMap((expr) => {
-    switch (expr.type) {
-      case "QueryBinary":
-        return [strFromBinary(expr)];
-      default:
-        return [];
-    }
-  });
-};
-
 const strFromContent = (queryContent: QueryContent): string | undefined => {
   const { content } = queryContent;
   switch (content.type) {
     case "QueryStr":
       return content.searchExpr;
     case "QueryGroup":
-      return `(${strFromList(content.content)})`;
+      return `(${strFromBinary(content.content).trim()})`;
     case "QueryBinary":
       return strFromBinary(content);
     default:
@@ -71,10 +63,10 @@ const strFromContent = (queryContent: QueryContent): string | undefined => {
 
 const strFromBinary = (queryBinary: QueryBinary): string => {
   const leftStr = strFromContent(queryBinary.left);
+
   const rightStr = queryBinary.right
-    ? `${queryBinary.right[0].tokenType.toString()} ${strFromBinary(
-        queryBinary.right[1]
-      )}`
+    ? `${queryBinary.right[0].lexeme} ${strFromBinary(queryBinary.right[1])}`
     : "";
-  return (leftStr ?? " ") + (rightStr ? ` ${rightStr}` : "");
+
+  return (leftStr ?? "") + (rightStr ? ` ${rightStr.trim()} ` : "");
 };