Repository: DestinyItemManager/DIM
Pull request #8204 (2022-03-20 23:06:40)

Search quotes
This adds the ability to escape quotes in strings, and builds out a better string quoter for use in both the canonicalizer and in search suggestions.


Patch:
diff --git a/src/app/compare/compare-buttons.tsx b/src/app/compare/compare-buttons.tsx
index 69226643f..b2245d8db 100644
--- a/src/app/compare/compare-buttons.tsx
+++ b/src/app/compare/compare-buttons.tsx
@@ -4,7 +4,7 @@ import { ArmorSlotIcon, WeaponSlotIcon, WeaponTypeIcon } from 'app/dim-ui/ItemCa
 import { SpecialtyModSlotIcon } from 'app/dim-ui/SpecialtyModSlotIcon';
 import { t } from 'app/i18next-t';
 import { DimItem } from 'app/inventory/item-types';
-import { quoteFilterString } from 'app/search/search-filters/freeform';
+import { quoteFilterString } from 'app/search/query-parser';
 import { getInterestingSocketMetadatas, getItemDamageShortName } from 'app/utils/item-utils';
 import { getWeaponArchetype } from 'app/utils/socket-utils';
 import rarityIcons from 'data/d2/engram-rarity-icons.json';
diff --git a/src/app/inventory/store/sockets.ts b/src/app/inventory/store/sockets.ts
index 5ff4ba7df..2642da1eb 100644
--- a/src/app/inventory/store/sockets.ts
+++ b/src/app/inventory/store/sockets.ts
@@ -377,8 +377,8 @@ function buildPlug(
 
   const failReasons = plug.enableFailIndexes
     ? _.compact(
-      plug.enableFailIndexes.map((index) => plugDef.plug!.enabledRules[index]?.failureMessage)
-    ).join('\n')
+        plug.enableFailIndexes.map((index) => plugDef.plug!.enabledRules[index]?.failureMessage)
+      ).join('\n')
     : '';
 
   return {
diff --git a/src/app/item-triage/ItemTriage.tsx b/src/app/item-triage/ItemTriage.tsx
index c397f4341..d63f0446d 100644
--- a/src/app/item-triage/ItemTriage.tsx
+++ b/src/app/item-triage/ItemTriage.tsx
@@ -18,8 +18,9 @@ import { DimPlugTooltip } from 'app/item-popup/PlugTooltip';
 import { editLoadout } from 'app/loadout-drawer/loadout-events';
 import { loadoutsSelector } from 'app/loadout-drawer/selectors';
 import { ItemFilter } from 'app/search/filter-types';
+import { quoteFilterString } from 'app/search/query-parser';
 import { filterFactorySelector } from 'app/search/search-filter';
-import { nameFilter, quoteFilterString } from 'app/search/search-filters/freeform';
+import { nameFilter } from 'app/search/search-filters/freeform';
 import {
   classFilter,
   damageFilter,
diff --git a/src/app/organizer/Columns.tsx b/src/app/organizer/Columns.tsx
index 909f4e199..1a22e4650 100644
--- a/src/app/organizer/Columns.tsx
+++ b/src/app/organizer/Columns.tsx
@@ -25,8 +25,8 @@ import { DimPlugTooltip } from 'app/item-popup/PlugTooltip';
 import RecoilStat, { recoilValue } from 'app/item-popup/RecoilStat';
 import { Loadout } from 'app/loadout-drawer/loadout-types';
 import { CUSTOM_TOTAL_STAT_HASH } from 'app/search/d2-known-values';
+import { quoteFilterString } from 'app/search/query-parser';
 import { statHashByName } from 'app/search/search-filter-values';
-import { quoteFilterString } from 'app/search/search-filters/freeform';
 import { getColor, percent } from 'app/shell/formatters';
 import {
   AppIcon,
diff --git a/src/app/search/query-parser.ts b/src/app/search/query-parser.ts
index 12b830f74..55e88cd6a 100644
--- a/src/app/search/query-parser.ts
+++ b/src/app/search/query-parser.ts
@@ -264,11 +264,6 @@ function isSameOp<T extends 'and' | 'or'>(binOp: T, op: QueryAST): op is AndOp |
 type NoArgTokenType = '(' | ')' | 'not' | 'or' | 'and' | 'implicit_and';
 export type Token = [NoArgTokenType] | ['filter', string, string] | ['comment', string];
 
-// Two different kind of quotes
-const quoteRegexes = {
-  '"': /.*?"/y,
-  "'": /.*?'/y,
-};
 // Parens: `(` can be followed by whitespace, while `)` can be preceded by it
 const parens = /(\(\s*|\s*\))/y;
 // A `-` followed by any amount of whitespace is the same as "not"
@@ -340,14 +335,34 @@ export function* lexer(query: string): Generator<Token> {
    * Consume and return the contents of a quoted string.
    */
   const consumeString = (startingQuoteChar: string) => {
+    const initial = i;
     // Quoted string
     consume(startingQuoteChar);
-    if ((match = extract(quoteRegexes[startingQuoteChar])) !== undefined) {
-      // Slice off the last character
-      return match.slice(0, match.length - 1);
-    } else {
-      throw new Error('Unterminated quotes: |' + query.slice(i) + '| ' + i);
+    let str = '';
+    while (i < query.length) {
+      const char = query[i];
+      consume(char);
+      // Handle character escapes e.g. \", \', \\
+      if (char === '\\') {
+        if (i < query.length) {
+          const escaped = query[i];
+          if (escaped === '"' || escaped === "'" || escaped === '\\') {
+            str += escaped;
+            consume(escaped);
+          } else {
+            throw new Error('Unrecognized escape sequence \\' + escaped);
+          }
+        } else {
+          str = str + char;
+        }
+      } else if (char === startingQuoteChar) {
+        return str;
+      } else {
+        str = str + char;
+      }
     }
+
+    throw new Error('Unterminated quotes: |' + query.slice(initial) + '| ' + initial);
   };
 
   while (i < query.length) {
@@ -399,6 +414,27 @@ export function* lexer(query: string): Generator<Token> {
   }
 }
 
+/**
+ * Quote a string if it's needed.
+ *
+ * @example
+ *
+ * quoteFilterString("foo bar") => "\"foo bar\""
+ * quoteFilterString("foobar") => "foobar"
+ * quoteFilterString("foo\"bar") => foobar"
+ */
+export function quoteFilterString(arg: string) {
+  const requiresQuotes = /[\s()"']/.test(arg);
+  if (requiresQuotes) {
+    const quoteChar = arg.includes('"') ? "'" : '"';
+    arg = arg.replaceAll('\\', '\\\\');
+    arg = arg.replaceAll(quoteChar, `\\${quoteChar}`);
+    return `${quoteChar}${arg}${quoteChar}`;
+  } else {
+    return arg;
+  }
+}
+
 /**
  * Build a standardized version of the query as a string. This is useful for deduping queries.
  * Example: 'is:weapon and is:sniperrifle or not is:armor and modslot:arrival' =>
@@ -409,8 +445,8 @@ 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}`;
+          ? quoteFilterString(query.args)
+          : `${query.type}:${quoteFilterString(query.args)}`;
       case 'not':
         return `-${canonicalizeQuery(query.operand, depth + 1)}`;
       case 'and':
diff --git a/src/app/search/search-filters/freeform.tsx b/src/app/search/search-filters/freeform.tsx
index df968b342..acb1865c8 100644
--- a/src/app/search/search-filters/freeform.tsx
+++ b/src/app/search/search-filters/freeform.tsx
@@ -6,6 +6,7 @@ import { DestinyInventoryItemDefinition } from 'bungie-api-ts/destiny2';
 import { ItemCategoryHashes, PlugCategoryHashes } from 'data/d2/generated-enums';
 import _ from 'lodash';
 import { FilterDefinition } from '../filter-types';
+import { quoteFilterString } from '../query-parser';
 
 /** global language bool. "latin" character sets are the main driver of string processing changes */
 const isLatinBased = (language: string) => DIM_LANG_INFOS[language].latinBased;
@@ -249,13 +250,3 @@ function getStringsFromAllSockets(item: DimItem, includeDescription = true) {
 
   return results;
 }
-
-// we can't properly quote a search string if it contains both ' and ", so.. we use this
-// to filter them out. small caveat there for the future "WHY DOESN'T THIS WORK" user
-export function isQuotable(s: string) {
-  return !(s.includes(`'`) && s.includes(`"`));
-}
-
-export function quoteFilterString(s: string) {
-  return s.includes(`"`) ? `'${s}'` : `"${s}"`;
-}
diff --git a/src/app/search/search-filters/loadouts.tsx b/src/app/search/search-filters/loadouts.tsx
index 8b0a9470b..0522c8ddc 100644
--- a/src/app/search/search-filters/loadouts.tsx
+++ b/src/app/search/search-filters/loadouts.tsx
@@ -1,7 +1,7 @@
 import { tl } from 'app/i18next-t';
 import { Loadout } from 'app/loadout-drawer/loadout-types';
 import { FilterDefinition } from '../filter-types';
-import { isQuotable, quoteFilterString } from './freeform';
+import { quoteFilterString } from '../query-parser';
 
 const loadoutFilters: FilterDefinition[] = [
   {
@@ -9,9 +9,7 @@ const loadoutFilters: FilterDefinition[] = [
     format: ['simple', 'freeform'],
 
     suggestionsGenerator: ({ loadouts }) =>
-      loadouts
-        ?.filter((l) => isQuotable(l.name))
-        .map((l) => 'inloadout:' + quoteFilterString(l.name.toLowerCase())),
+      loadouts?.map((l) => 'inloadout:' + quoteFilterString(l.name.toLowerCase())),
 
     description: tl('Filter.InLoadout'),
     filter: ({ filterValue, loadouts }) => {


Test patch:
diff --git a/src/app/search/query-parser.test.ts b/src/app/search/query-parser.test.ts
index b8525efc5..09b42dfee 100644
--- a/src/app/search/query-parser.test.ts
+++ b/src/app/search/query-parser.test.ts
@@ -1,4 +1,4 @@
-import { canonicalizeQuery, lexer, parseQuery, Token } from './query-parser';
+import { canonicalizeQuery, lexer, parseQuery, quoteFilterString, Token } from './query-parser';
 
 // To update the snapshots, run:
 // npx jest --updateSnapshot src/app/search/query-parser.test.ts
@@ -109,6 +109,23 @@ const canonicalize = [
     '/* 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")',
   ],
+  ['inloadout:"----<()>fast"', 'inloadout:"----<()>fast"'],
+  ['perkname:"foobar"', 'perkname:foobar'],
+  ["perkname:'foo bar'", 'perkname:"foo bar"'],
+  ['perkname:"foobar"', 'perkname:foobar'],
+  ["perkname:'foo\"bar'", "perkname:'foo\"bar'"],
+  ['perkname:"foo\\"bar"', "perkname:'foo\"bar'"],
+  ["perkname:'foo\\\"ba\\'r'", "perkname:'foo\"ba\\'r'"],
+];
+
+// Test that we can quote a string, parse it back as part of a search, and get the original string
+const quotes = [
+  ['foobar'],
+  ['Foo\\bar'],
+  ['My cool loadout'],
+  ['My "cool" loadout'],
+  ['My "cool" loadout\'s little brother'],
+  ['My "cool" load\\out\'s little brother'],
 ];
 
 test.each(cases)('parse |%s|', (query) => {
@@ -134,3 +151,13 @@ test.each(canonicalize)('|%s| is canonically |%s|', (query, canonical) => {
   const canonicalized = canonicalizeQuery(parseQuery(query));
   expect(canonicalized).toEqual(canonical);
 });
+
+test.each(quotes)('|%s| quoting roundtrip', (str) => {
+  const quoted = quoteFilterString(str);
+  const ast = parseQuery(`name:${quoted}`);
+  if (ast.op === 'filter') {
+    expect(ast.args).toEqual(str.toLowerCase());
+  } else {
+    throw new Error(`Failed: ${quoted}`);
+  }
+});