Repository: DestinyItemManager/DIM (ts)

Issue:
Named Filters
### Proposed change

Filters are great, but they can be cumbersome if you use a long filter copied in from a text file. I propose adding the ability to add a name to a filter. If a filter has a name, it gets displayed in the dropdown list as a named filter, rather than as the raw filter itself.

While this wouldn't be useful for short, simple filters, it'd be fantastic for those filters we build to sort through junk armor and the like, as they can be several lines long.

### How does this fit into your workflow?

I use DIM on desktop and tablet and this kind of change would make it much easier to use filters everywhere, but they'd be particularly useful on a tablet where it's harder to interact with text and where screen space is limited. With named filters it would be very easy to add complex filters using a desktop, name them, and then have a short and easy to interpret list when I move over to my tablet.

Since complex filters can easily take up three lines, this change would simplify the filter dropdown, making it much more readable during normal operations.

Pull request:
Support comments in searches
Fixes #7174 to the extent I want to fix it.

This allows for single-line comments of the form `/* A comment */` in searches. When canonicalizing, all but the top-level comment will be removed, but the top level comment will always be in front. This is mostly an alternative to doing `name:"My cool search" or ...`

Patch:
diff --git a/src/app/search/query-parser.ts b/src/app/search/query-parser.ts
index 9bfbac485..12b830f74 100644
--- a/src/app/search/query-parser.ts
+++ b/src/app/search/query-parser.ts
@@ -26,6 +26,7 @@
  */
 export type QueryAST = (AndOp | OrOp | NotOp | FilterOp | NoOp) & {
   error?: Error;
+  comment?: string;
 };
 
 /** If ALL of of the operands are true, this resolves to true. There may be any number of operands. */
@@ -179,6 +180,14 @@ export function parseQuery(query: string): QueryAST {
         }
         return result;
       }
+      case 'comment': {
+        const comment = token[1];
+        const next = parseAtom(tokens);
+        return {
+          ...next,
+          comment: comment,
+        };
+      }
       default:
         throw new Error('Unexpected token type, looking for an atom: ' + token + ', ' + query);
     }
@@ -215,10 +224,15 @@ export function parseQuery(query: string): QueryAST {
         if (isSameOp(operator.op, ast)) {
           ast.operands.push(rhs);
         } else {
+          const title = ast.comment;
+          delete ast.comment;
           ast = {
             op: operator.op,
             operands: isSameOp(operator.op, rhs) ? [ast, ...rhs.operands] : [ast, rhs],
           };
+          if (title) {
+            ast.comment = title;
+          }
         }
       }
     } catch (e) {
@@ -248,7 +262,7 @@ function isSameOp<T extends 'and' | 'or'>(binOp: T, op: QueryAST): op is AndOp |
 
 // Lexer token types
 type NoArgTokenType = '(' | ')' | 'not' | 'or' | 'and' | 'implicit_and';
-export type Token = [NoArgTokenType] | ['filter', string, string];
+export type Token = [NoArgTokenType] | ['filter', string, string] | ['comment', string];
 
 // Two different kind of quotes
 const quoteRegexes = {
@@ -270,6 +284,7 @@ const filterArgs = /[^\s()]+/y;
 const bareWords = /[^\s)]+/y;
 // Whitespace that doesn't match anything else is an implicit `and`
 const whitespace = /\s+/y;
+const comment = /\/\*(.*?)\*\/\s*/y;
 
 /**
  * The lexer yields a series of tokens representing the linear structure of the search query.
@@ -315,7 +330,7 @@ export function* lexer(query: string): Generator<Token> {
       const result = match[0];
       if (result.length > 0) {
         consume(result);
-        return result;
+        return match.length > 1 ? match[1] : result;
       }
     }
     return undefined;
@@ -351,6 +366,8 @@ export function* lexer(query: string): Generator<Token> {
     } else if ((match = extract(booleanKeywords)) !== undefined) {
       // boolean keywords
       yield [match.trim() as NoArgTokenType];
+    } else if ((match = extract(comment)) !== undefined) {
+      yield ['comment', match.trim()];
     } else if ((match = extract(filterName)) !== undefined) {
       // Keyword searches - is:, stat:discipline:, etc
       const keyword = match.slice(0, match.length - 1);
@@ -388,26 +405,35 @@ export function* lexer(query: string): Generator<Token> {
  *          '(-is:armor modslot:arrival) or (is:sniperrifle is:weapon)'
  */
 export function canonicalizeQuery(query: QueryAST, depth = 0): string {
-  switch (query.op) {
-    case 'filter':
-      return query.type === 'keyword'
-        ? `${/\s/.test(query.args) ? `"${query.args}"` : query.args}`
-        : `${query.type}:${/\s/.test(query.args) ? `"${query.args}"` : query.args}`;
-    case 'not':
-      return `-${canonicalizeQuery(query.operand, depth + 1)}`;
-    case 'and':
-    case 'or': {
-      const joinedOperands = query.operands
-        .map((q) => canonicalizeQuery(q, depth + 1))
-        .join(
-          query.op === 'and' &&
-            !query.operands.some((op) => op.op === 'filter' && op.type === 'keyword')
-            ? ' '
-            : ` ${query.op} `
-        );
-      return depth === 0 ? joinedOperands : `(${joinedOperands})`;
+  const result = (() => {
+    switch (query.op) {
+      case 'filter':
+        return query.type === 'keyword'
+          ? `${/\s/.test(query.args) ? `"${query.args}"` : query.args}`
+          : `${query.type}:${/\s/.test(query.args) ? `"${query.args}"` : query.args}`;
+      case 'not':
+        return `-${canonicalizeQuery(query.operand, depth + 1)}`;
+      case 'and':
+      case 'or': {
+        const joinedOperands = query.operands
+          .map((q) => canonicalizeQuery(q, depth + 1))
+          .join(
+            query.op === 'and' &&
+              !query.operands.some((op) => op.op === 'filter' && op.type === 'keyword')
+              ? ' '
+              : ` ${query.op} `
+          );
+        return depth === 0 ? joinedOperands : `(${joinedOperands})`;
+      }
+      case 'noop':
+        return '';
     }
-    case 'noop':
-      return '';
+  })();
+
+  // Only preserve the top-level comment
+  if (query.comment && depth === 0) {
+    return `/* ${query.comment} */ ${result}`;
   }
+
+  return result;
 }


Test patch:
diff --git a/src/app/search/__snapshots__/query-parser.test.ts.snap b/src/app/search/__snapshots__/query-parser.test.ts.snap
index a6b6891df..227a6d275 100644
--- a/src/app/search/__snapshots__/query-parser.test.ts.snap
+++ b/src/app/search/__snapshots__/query-parser.test.ts.snap
@@ -3180,6 +3180,31 @@ Array [
 ]
 `;
 
+exports[`parse |  /* My cool search */
+ is:armor|: ast 1`] = `
+Object {
+  "args": "armor",
+  "comment": "my cool search",
+  "op": "filter",
+  "type": "is",
+}
+`;
+
+exports[`parse |  /* My cool search */
+ is:armor|: lexer 1`] = `
+Array [
+  Array [
+    "comment",
+    "my cool search",
+  ],
+  Array [
+    "filter",
+    "is",
+    "armor",
+  ],
+]
+`;
+
 exports[`parse |"grenade launcher reserves"|: ast 1`] = `
 Object {
   "args": "grenade launcher reserves",
@@ -3588,6 +3613,127 @@ Array [
 ]
 `;
 
+exports[`parse |/* My cool search */ (/* armor */ is:armor and is:blue) or (/*weapons*/ is:weapon and perkname:"Kill Clip")|: ast 1`] = `
+Object {
+  "comment": "my cool search",
+  "op": "or",
+  "operands": Array [
+    Object {
+      "op": "and",
+      "operands": Array [
+        Object {
+          "args": "armor",
+          "op": "filter",
+          "type": "is",
+        },
+        Object {
+          "args": "blue",
+          "op": "filter",
+          "type": "is",
+        },
+      ],
+    },
+    Object {
+      "comment": "weapons",
+      "op": "and",
+      "operands": Array [
+        Object {
+          "args": "weapon",
+          "op": "filter",
+          "type": "is",
+        },
+        Object {
+          "args": "kill clip",
+          "op": "filter",
+          "type": "perkname",
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`parse |/* My cool search */ (/* armor */ is:armor and is:blue) or (/*weapons*/ is:weapon and perkname:"Kill Clip")|: lexer 1`] = `
+Array [
+  Array [
+    "comment",
+    "my cool search",
+  ],
+  Array [
+    "(",
+  ],
+  Array [
+    "comment",
+    "armor",
+  ],
+  Array [
+    "filter",
+    "is",
+    "armor",
+  ],
+  Array [
+    "and",
+  ],
+  Array [
+    "filter",
+    "is",
+    "blue",
+  ],
+  Array [
+    ")",
+  ],
+  Array [
+    "or",
+  ],
+  Array [
+    "(",
+  ],
+  Array [
+    "comment",
+    "weapons",
+  ],
+  Array [
+    "filter",
+    "is",
+    "weapon",
+  ],
+  Array [
+    "and",
+  ],
+  Array [
+    "filter",
+    "perkname",
+    "kill clip",
+  ],
+  Array [
+    ")",
+  ],
+]
+`;
+
+exports[`parse |/* My cool search */ is:armor|: ast 1`] = `
+Object {
+  "args": "armor",
+  "comment": "my cool search",
+  "op": "filter",
+  "type": "is",
+}
+`;
+
+exports[`parse |/* My cool search */ is:armor|: lexer 1`] = `
+Array [
+  Array [
+    "comment",
+    "my cool search",
+  ],
+  Array [
+    "filter",
+    "is",
+    "armor",
+  ],
+]
+`;
+
 exports[`parse |- is:exotic - (power:>1000)|: ast 1`] = `
 Object {
   "op": "and",
diff --git a/src/app/search/query-parser.test.ts b/src/app/search/query-parser.test.ts
index 1eb90dc71..b8525efc5 100644
--- a/src/app/search/query-parser.test.ts
+++ b/src/app/search/query-parser.test.ts
@@ -55,6 +55,12 @@ const cases = [
   ['not (forgotten)'],
   ['not "forgotten"'],
   ['gnawing hunger'],
+  // Comments
+  ['/* My cool search */ is:armor'],
+  ['  /* My cool search */\n is:armor'],
+  [
+    '/* My cool search */ (/* armor */ is:armor and is:blue) or (/*weapons*/ is:weapon and perkname:"Kill Clip")',
+  ],
 ];
 
 // Each of these asserts that the first query is the same as the second query once parsed
@@ -98,6 +104,11 @@ const canonicalize = [
   ],
   ['( power:>1000 and -modslot:arrival ) ', 'power:>1000 -modslot:arrival'],
   ['food fight', 'food and fight'],
+  ['/* My cool search   */\n is:armor', '/* my cool search */ is:armor'],
+  [
+    '/* My cool search */ (/* armor */ is:armor and is:blue) or (/*weapons*/ is:weapon and perkname:"Kill Clip")',
+    '/* my cool search */ (is:armor is:blue) or (is:weapon perkname:"kill clip")',
+  ],
 ];
 
 test.each(cases)('parse |%s|', (query) => {