Permit QueryFields anywhere in the query, not just at the root, align…

…ing the grammar with the blog post 😅
guardian · Oct 6, 2024 · f89645f · f89645f
1 parent db35406
commit f89645f
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 41 deletions.
diff --git a/prosemirror-client/src/lang/ast.ts b/prosemirror-client/src/lang/ast.ts
@@ -2,7 +2,7 @@ import { Token } from "./token";
 
 export type QueryList = {
   type: "QueryList";
-  content: (QueryBinary | QueryField)[];
+  content: QueryBinary[];
 };
 
 export const createQueryList = (
@@ -31,7 +31,7 @@ export const createQueryBinary = (
 
 export type QueryContent = {
   type: "QueryContent";
-  content: QueryStr | QueryBinary | QueryGroup;
+  content: QueryStr | QueryBinary | QueryGroup | QueryField;
 };
 
 export const createQueryContent = (

diff --git a/prosemirror-client/src/lang/capiQueryString.ts b/prosemirror-client/src/lang/capiQueryString.ts
@@ -1,5 +1,6 @@
 import { err, ok, Result } from "../util/result";
 import { QueryBinary, QueryContent, QueryList } from "./ast";
+import { getQueryFieldsFromQueryList } from "./util";
 
 class CapiQueryStringError extends Error {
   public constructor(message: string) {
@@ -20,34 +21,38 @@ export const queryStrFromQueryList = (
   });
 
   try {
-    const otherQueries = program.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)`
-            );
+    const otherQueries = getQueryFieldsFromQueryList(program).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 maybeSearchStr = searchStrs.length
-      ? `q=${encodeURI(searchStrs.join(" "))}`
+    const searchStr = maybeSearchStr.length
+      ? `q=${encodeURI(maybeSearchStr)}`
       : "";
 
-    return ok([maybeSearchStr, otherQueries].filter(Boolean).flat().join("&"));
+    return ok([searchStr, otherQueries].filter(Boolean).flat().join("&"));
   } catch (e) {
     return err(e as Error);
   }
 };
 
-const strFromContent = (queryContent: QueryContent): string => {
+const strFromContent = (queryContent: QueryContent): string | undefined => {
   const { content } = queryContent;
   switch (content.type) {
     case "QueryStr":
@@ -56,15 +61,18 @@ const strFromContent = (queryContent: QueryContent): string => {
       return `(${strFromBinary(content.content)})`;
     case "QueryBinary":
       return strFromBinary(content);
+    default:
+      // Ignore fields
+      return;
   }
 };
 
 const strFromBinary = (queryBinary: QueryBinary): string => {
   const leftStr = strFromContent(queryBinary.left);
   const rightStr = queryBinary.right
-    ? ` ${queryBinary.right[0].tokenType.toString()} ${strFromBinary(
+    ? `${queryBinary.right[0].tokenType.toString()} ${strFromBinary(
         queryBinary.right[1]
       )}`
     : "";
-  return leftStr + rightStr;
+  return (leftStr ?? " ") + (rightStr ? ` ${rightStr}` : "");
 };
diff --git a/prosemirror-client/src/lang/parser.spec.ts b/prosemirror-client/src/lang/parser.spec.ts
@@ -60,7 +60,13 @@ describe("parser", () => {
       eofToken(4),
     ];
     const result = new Parser(tokens).parse();
-    expect(result).toEqual(ok(createQueryList([queryField("ta", "")])));
+    expect(result).toEqual(
+      ok(
+        createQueryList([
+          createQueryBinary(createQueryContent(queryField("ta", ""))),
+        ])
+      )
+    );
   });
 
   it("should handle an unbalanced binary", () => {
@@ -114,7 +120,12 @@ describe("parser", () => {
       ok(
         createQueryList([
           createQueryBinary(createQueryContent(createQueryStr("a")), undefined),
-          createQueryField(queryFieldKeyToken("", 2), undefined),
+          createQueryBinary(
+            createQueryContent(
+              createQueryField(queryFieldKeyToken("", 2), undefined)
+            ),
+            undefined
+          ),
         ])
       )
     );

diff --git a/prosemirror-client/src/lang/parser.ts b/prosemirror-client/src/lang/parser.ts
@@ -33,7 +33,7 @@ export class Parser {
 
   private QueryList() {
     try {
-      const queries: (QueryBinary | QueryField)[] = [];
+      const queries: QueryBinary[] = [];
       while (this.peek().tokenType !== TokenType.EOF) {
         queries.push(this.query());
       }
@@ -50,10 +50,8 @@ export class Parser {
   private startOfQueryField = [TokenType.QUERY_FIELD_KEY, TokenType.PLUS];
   private startOfQueryValue = [TokenType.QUERY_VALUE, TokenType.COLON];
 
-  private query(): QueryBinary | QueryField {
-    if (this.startOfQueryField.some((i) => i === this.peek().tokenType)) {
-      return this.queryField();
-    } else if (this.startOfQueryValue.some((i) => i === this.peek().tokenType))
+  private query(): QueryBinary {
+    if (this.startOfQueryValue.some((i) => i === this.peek().tokenType))
       throw new ParseError(
         this.peek().start,
         "I found an unexpected ':'. Did you numberend to search for a tag, section or similar, e.g. tag:news? If you would like to add a search phrase containing a ':' character, please surround it in double quotes."
@@ -103,6 +101,10 @@ export class Parser {
           throw this.error(
             `An ${tokenType.toString()} keyword must have a search term before and after it, e.g. this ${tokenType.toString()} that.`
           );
+        } else if (
+          this.startOfQueryField.some((i) => i === this.peek().tokenType)
+        ) {
+          return createQueryContent(this.queryField());
         } else {
           throw this.error(
             `I didn't expect what I found after '${this.previous()?.lexeme}'`

diff --git a/prosemirror-client/src/lang/typeahead.ts b/prosemirror-client/src/lang/typeahead.ts
@@ -1,4 +1,4 @@
-import { QueryList, QueryField } from "./ast";
+import { QueryList, QueryField, QueryContent, QueryBinary } from "./ast";
 import { Token } from "./token";
 import {
   DateSuggestion,
@@ -7,6 +7,7 @@ import {
   TypeaheadSuggestion,
   TypeaheadType,
 } from "./types";
+import { getQueryFieldsFromQueryList } from "./util";
 
 export type TypeaheadResolver =
   | ((str: string, signal?: AbortSignal) => Promise<TextSuggestionOption[]>)
@@ -52,18 +53,9 @@ export class Typeahead {
     program: QueryList,
     signal?: AbortSignal
   ): Promise<TypeaheadSuggestion[]> {
-    const suggestions = program.content
-      .map((expr) => {
-        switch (expr.type) {
-          case "QueryField": {
-            return this.suggestQueryField(expr, signal);
-          }
-          default: {
-            return Promise.resolve([]);
-          }
-        }
-      })
-      .flat();
+    const suggestions = getQueryFieldsFromQueryList(program).flatMap(
+      (queryField) => this.suggestQueryField(queryField, signal)
+    );
 
     return Promise.all(suggestions).then((suggestions) => suggestions.flat());
   }

diff --git a/prosemirror-client/src/lang/util.ts b/prosemirror-client/src/lang/util.ts
@@ -1,3 +1,5 @@
+import { QueryBinary, QueryContent, QueryField, QueryList } from "./ast";
+
 const whitespaceR = /\s/;
 export const isWhitespace = (str: string) => whitespaceR.test(str);
 
@@ -31,3 +33,25 @@ export function* getPermutations<T>(
 
   return permutation.slice();
 }
+
+export const getQueryFieldsFromQueryList = (
+  queryList: QueryList
+): QueryField[] => queryList.content.flatMap(getQueryFieldsFromQueryBinary);
+
+export const getQueryFieldsFromQueryBinary = (
+  queryBinary: QueryBinary
+): QueryField[] =>
+  getQueryFieldsFromQueryContent(queryBinary.left).concat(
+    queryBinary.right ? getQueryFieldsFromQueryBinary(queryBinary.right[1]) : []
+  );
+
+export const getQueryFieldsFromQueryContent = (
+  queryContent: QueryContent
+): QueryField[] => {
+  switch (queryContent.content.type) {
+    case "QueryField":
+      return [queryContent.content];
+    default:
+      return [];
+  }
+};