Compact tags refactor (#1021)

* Update translator to flag redundant tags instead of remove

* Update how compact tags are shown in the popup

* Pass compactTags option to note builder

* Update options templates

* Add options upgrade

* Add options upgrade test

10 files changed, 153 insertions, 38 deletions

diff --git a/ext/bg/data/default-anki-field-templates.handlebars b/ext/bg/data/default-anki-field-templates.handlebars
index 776792c7..0b0e9ca6 100644
--- a/ext/bg/data/default-anki-field-templates.handlebars
+++ b/ext/bg/data/default-anki-field-templates.handlebars
@@ -1,6 +1,17 @@
 {{#*inline "glossary-single"}}
     {{~#unless brief~}}
-        {{~#if definitionTags~}}<i>({{#each definitionTags}}{{name}}{{#unless @last}}, {{/unless}}{{/each}})</i> {{/if~}}
+        {{~#scope~}}
+            {{~#set "any" false}}{{/set~}}
+            {{~#if definitionTags~}}{{#each definitionTags~}}
+                {{~#if (op "||" (op "!" ../data.compactTags) (op "!" redundant))~}}
+                    {{~#if (get "any")}}, {{else}}<i>({{/if~}}
+                    {{name}}
+                    {{~#set "any" true}}{{/set~}}
+                {{~/if~}}
+            {{~/each~}}
+            {{~#if (get "any")}})</i> {{/if~}}
+            {{~/if~}}
+        {{~/scope~}}
         {{~#if only~}}({{#each only}}{{{.}}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
     {{~/unless~}}
     {{~#if glossary.[1]~}}
@@ -92,18 +103,18 @@
     {{~else~}}
         {{~#if group~}}
             {{~#if definition.definitions.[1]~}}
-                <ol>{{#each definition.definitions}}<li>{{> glossary-single brief=../brief compactGlossaries=../compactGlossaries}}</li>{{/each}}</ol>
+                <ol>{{#each definition.definitions}}<li>{{> glossary-single brief=../brief compactGlossaries=../compactGlossaries data=../.}}</li>{{/each}}</ol>
             {{~else~}}
-                {{~> glossary-single definition.definitions.[0] brief=brief compactGlossaries=compactGlossaries~}}
+                {{~> glossary-single definition.definitions.[0] brief=brief compactGlossaries=compactGlossaries data=.~}}
             {{~/if~}}
         {{~else if merge~}}
             {{~#if definition.definitions.[1]~}}
-                <ol>{{#each definition.definitions}}<li>{{> glossary-single brief=../brief compactGlossaries=../compactGlossaries}}</li>{{/each}}</ol>
+                <ol>{{#each definition.definitions}}<li>{{> glossary-single brief=../brief compactGlossaries=../compactGlossaries data=../.}}</li>{{/each}}</ol>
             {{~else~}}
-                {{~> glossary-single definition.definitions.[0] brief=brief compactGlossaries=compactGlossaries~}}
+                {{~> glossary-single definition.definitions.[0] brief=brief compactGlossaries=compactGlossaries data=.~}}
             {{~/if~}}
         {{~else~}}
-            {{~> glossary-single definition brief=brief compactGlossaries=compactGlossaries~}}
+            {{~> glossary-single definition brief=brief compactGlossaries=compactGlossaries data=.~}}
         {{~/if~}}
     {{~/if~}}
     </div>
diff --git a/ext/bg/js/anki-note-builder.js b/ext/bg/js/anki-note-builder.js
index 4ac597da..d1e918c9 100644
--- a/ext/bg/js/anki-note-builder.js
+++ b/ext/bg/js/anki-note-builder.js
@@ -35,6 +35,7 @@ class AnkiNoteBuilder {
         duplicateScope='collection',
         resultOutputMode='split',
         compactGlossaries=false,
+        compactTags=false,
         modeOptions: {fields, deck, model},
         audioDetails=null,
         screenshotDetails=null,
@@ -70,7 +71,7 @@ class AnkiNoteBuilder {
             }
         };
 
-        const data = this._createNoteData(definition, mode, context, resultOutputMode, compactGlossaries);
+        const data = this._createNoteData(definition, mode, context, resultOutputMode, compactGlossaries, compactTags);
         const formattedFieldValuePromises = [];
         for (const [, fieldValue] of fieldEntries) {
             const formattedFieldValuePromise = this._formatField(fieldValue, data, templates, errors);
@@ -104,7 +105,7 @@ class AnkiNoteBuilder {
 
     // Private
 
-    _createNoteData(definition, mode, context, resultOutputMode, compactGlossaries) {
+    _createNoteData(definition, mode, context, resultOutputMode, compactGlossaries, compactTags) {
         const pitches = DictionaryDataUtil.getPitchAccentInfos(definition);
         const pitchCount = pitches.reduce((i, v) => i + v.pitches.length, 0);
         return {
@@ -118,6 +119,7 @@ class AnkiNoteBuilder {
             modeTermKana: mode === 'term-kana',
             modeKanji: mode === 'kanji',
             compactGlossaries,
+            compactTags,
             context
         };
     }
diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js
index d1d25153..c3e3339c 100644
--- a/ext/bg/js/backend.js
+++ b/ext/bg/js/backend.js
@@ -1650,7 +1650,7 @@ class Backend {
         const {wildcard} = details;
         const enabledDictionaryMap = this._getTranslatorEnabledDictionaryMap(options);
         const {
-            general: {compactTags, mainDictionary},
+            general: {mainDictionary},
             scanning: {alphanumeric},
             translation: {
                 convertHalfWidthCharacters,
@@ -1663,7 +1663,6 @@ class Backend {
         } = options;
         return {
             wildcard,
-            compactTags,
             mainDictionary,
             alphanumeric,
             convertHalfWidthCharacters,
diff --git a/ext/bg/js/options.js b/ext/bg/js/options.js
index bb90e655..e4f6c8e4 100644
--- a/ext/bg/js/options.js
+++ b/ext/bg/js/options.js
@@ -618,7 +618,35 @@ class OptionsUtil {
         options.global.showPopupPreview = false;
         for (const profile of options.profiles) {
             profile.options.anki.checkForDuplicates = true;
+            const fieldTemplates = profile.options.anki.fieldTemplates;
+            if (typeof fieldTemplates === 'string') {
+                profile.options.anki.fieldTemplates = this._updateVersion6AnkiTemplatesCompactTags(fieldTemplates);
+            }
         }
         return options;
     }
+
+    _updateVersion6AnkiTemplatesCompactTags(templates) {
+        const rawPattern1 = '{{~#if definitionTags~}}<i>({{#each definitionTags}}{{name}}{{#unless @last}}, {{/unless}}{{/each}})</i> {{/if~}}';
+        const pattern1 = new RegExp(`((\r?\n)?[ \t]*)${escapeRegExp(rawPattern1)}`, 'g');
+        const replacement1 = (
+        // eslint-disable-next-line indent
+`{{~#scope~}}
+    {{~#set "any" false}}{{/set~}}
+    {{~#if definitionTags~}}{{#each definitionTags~}}
+        {{~#if (op "||" (op "!" ../data.compactTags) (op "!" redundant))~}}
+            {{~#if (get "any")}}, {{else}}<i>({{/if~}}
+            {{name}}
+            {{~#set "any" true}}{{/set~}}
+        {{~/if~}}
+    {{~/each~}}
+    {{~#if (get "any")}})</i> {{/if~}}
+    {{~/if~}}
+{{~/scope~}}`
+        );
+        const simpleNewline = /\n/g;
+        templates = templates.replace(pattern1, (g0, space) => (space + replacement1.replace(simpleNewline, space)));
+        templates = templates.replace(/\bcompactGlossaries=((?:\.*\/)*)compactGlossaries\b/g, (g0, g1) => `${g0} data=${g1}.`);
+        return templates;
+    }
 }
diff --git a/ext/bg/js/settings/anki-templates-controller.js b/ext/bg/js/settings/anki-templates-controller.js
index e6bab256..e1e01627 100644
--- a/ext/bg/js/settings/anki-templates-controller.js
+++ b/ext/bg/js/settings/anki-templates-controller.js
@@ -183,7 +183,7 @@ class AnkiTemplatesController {
                 const ankiNoteBuilder = new AnkiNoteBuilder({
                     renderTemplate: this._renderTemplate.bind(this)
                 });
-                const {general: {resultOutputMode, compactGlossaries}} = options;
+                const {general: {resultOutputMode, compactGlossaries, compactTags}} = options;
                 const note = await ankiNoteBuilder.createNote({
                     definition,
                     mode,
@@ -191,6 +191,7 @@ class AnkiTemplatesController {
                     templates,
                     resultOutputMode,
                     compactGlossaries,
+                    compactTags,
                     modeOptions: {
                         fields: {field},
                         deck: '',
diff --git a/ext/bg/js/translator.js b/ext/bg/js/translator.js
index 6e29aeae..2c4d7d3a 100644
--- a/ext/bg/js/translator.js
+++ b/ext/bg/js/translator.js
@@ -59,7 +59,6 @@ class Translator {
      * @param options An object using the following structure:
      *   {
      *     wildcard: (null or string),
-     *     compactTags: (boolean),
      *     mainDictionary: (string),
      *     alphanumeric: (boolean),
      *     convertHalfWidthCharacters: (enum: 'false', 'true', 'variant'),
@@ -156,24 +155,22 @@ class Translator {
     }
 
     async _findTermsGrouped(text, options) {
-        const {compactTags, enabledDictionaryMap} = options;
+        const {enabledDictionaryMap} = options;
         const [definitions, length] = await this._findTermsInternal(text, enabledDictionaryMap, options);
 
         const groupedDefinitions = this._groupTerms(definitions, enabledDictionaryMap);
         await this._buildTermMeta(groupedDefinitions, enabledDictionaryMap);
         this._sortDefinitions(groupedDefinitions, false);
 
-        if (compactTags) {
-            for (const definition of groupedDefinitions) {
-                this._compressDefinitionTags(definition.definitions);
-            }
+        for (const definition of groupedDefinitions) {
+            this._flagRedundantDefinitionTags(definition.definitions);
         }
 
         return [groupedDefinitions, length];
     }
 
     async _findTermsMerged(text, options) {
-        const {compactTags, mainDictionary, enabledDictionaryMap} = options;
+        const {mainDictionary, enabledDictionaryMap} = options;
         const secondarySearchDictionaryMap = this._getSecondarySearchDictionaryMap(enabledDictionaryMap);
 
         const [definitions, length] = await this._findTermsInternal(text, enabledDictionaryMap, options);
@@ -212,10 +209,8 @@ class Translator {
         await this._buildTermMeta(definitionsMerged, enabledDictionaryMap);
         this._sortDefinitions(definitionsMerged, false);
 
-        if (compactTags) {
-            for (const definition of definitionsMerged) {
-                this._compressDefinitionTags(definition.definitions);
-            }
+        for (const definition of definitionsMerged) {
+            this._flagRedundantDefinitionTags(definition.definitions);
         }
 
         return [definitionsMerged, length];
@@ -541,7 +536,7 @@ class Translator {
         }
     }
 
-    _compressDefinitionTags(definitions) {
+    _flagRedundantDefinitionTags(definitions) {
         let lastDictionary = '';
         let lastPartOfSpeech = '';
         const removeCategoriesSet = new Set();
@@ -564,7 +559,7 @@ class Translator {
             }
 
             if (removeCategoriesSet.size > 0) {
-                this._removeTagsWithCategory(definitionTags, removeCategoriesSet);
+                this._flagTagsWithCategoryAsRedundant(definitionTags, removeCategoriesSet);
                 removeCategoriesSet.clear();
             }
         }
@@ -749,7 +744,7 @@ class Translator {
             const meta = tagMetaList[i];
             const name = names[i];
             const {category, notes, order, score} = (meta !== null ? meta : {});
-            const tag = this._createTag(name, category, notes, order, score, dictionary);
+            const tag = this._createTag(name, category, notes, order, score, dictionary, false);
             results.push(tag);
         }
         return results;
@@ -893,13 +888,11 @@ class Translator {
         return results;
     }
 
-    _removeTagsWithCategory(tags, removeCategoriesSet) {
-        for (let i = 0, ii = tags.length; i < ii; ++i) {
-            const {category} = tags[i];
-            if (!removeCategoriesSet.has(category)) { continue; }
-            tags.splice(i, 1);
-            --i;
-            --ii;
+    _flagTagsWithCategoryAsRedundant(tags, removeCategoriesSet) {
+        for (const tag of tags) {
+            if (removeCategoriesSet.has(tag.category)) {
+                tag.redundant = true;
+            }
         }
     }
 
@@ -970,8 +963,8 @@ class Translator {
     // Common data creation and cloning functions
 
     _cloneTag(tag) {
-        const {name, category, notes, order, score, dictionary} = tag;
-        return this._createTag(name, category, notes, order, score, dictionary);
+        const {name, category, notes, order, score, dictionary, redundant} = tag;
+        return this._createTag(name, category, notes, order, score, dictionary, redundant);
     }
 
     _cloneTags(tags) {
@@ -987,17 +980,18 @@ class Translator {
     }
 
     _createDictionaryTag(name) {
-        return this._createTag(name, 'dictionary', '', 100, 0, name);
+        return this._createTag(name, 'dictionary', '', 100, 0, name, false);
     }
 
-    _createTag(name, category, notes, order, score, dictionary) {
+    _createTag(name, category, notes, order, score, dictionary, redundant) {
         return {
             name,
             category: (typeof category === 'string' && category.length > 0 ? category : 'default'),
             notes: (typeof notes === 'string' ? notes : ''),
             order: (typeof order === 'number' ? order : 0),
             score: (typeof score === 'number' ? score : 0),
-            dictionary: (typeof dictionary === 'string' ? dictionary : null)
+            dictionary: (typeof dictionary === 'string' ? dictionary : null),
+            redundant
         };
     }
 
diff --git a/ext/mixed/css/display.css b/ext/mixed/css/display.css
index 4914bfa1..77f6b073 100644
--- a/ext/mixed/css/display.css
+++ b/ext/mixed/css/display.css
@@ -562,6 +562,10 @@ button.action-button {
     display: inline;
 }
 
+:root[data-compact-tags=true] .tag[data-redundant=true] {
+    display: none;
+}
+
 .term-glossary-separator,
 .term-reason-separator {
     display: inline;
diff --git a/ext/mixed/js/display-generator.js b/ext/mixed/js/display-generator.js
index 910d45da..53d68162 100644
--- a/ext/mixed/js/display-generator.js
+++ b/ext/mixed/js/display-generator.js
@@ -349,6 +349,7 @@ class DisplayGenerator {
         node.title = details.notes;
         inner.textContent = details.name;
         node.dataset.category = details.category;
+        if (details.redundant) { node.dataset.redundant = true; }
 
         return node;
     }
diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js
index 0d3ee69d..69d94603 100644
--- a/ext/mixed/js/display.js
+++ b/ext/mixed/js/display.js
@@ -726,6 +726,7 @@ class Display extends EventDispatcher {
         data.ankiEnabled = `${options.anki.enable}`;
         data.audioEnabled = `${options.audio.enabled && options.audio.sources.length > 0}`;
         data.compactGlossaries = `${options.general.compactGlossaries}`;
+        data.compactTags = `${options.general.compactTags}`;
         data.enableSearchTags = `${options.scanning.enableSearchTags}`;
         data.showPitchAccentDownstepNotation = `${options.general.showPitchAccentDownstepNotation}`;
         data.showPitchAccentPositionNotation = `${options.general.showPitchAccentPositionNotation}`;
@@ -1421,7 +1422,7 @@ class Display extends EventDispatcher {
 
     async _createNote(definition, mode, context, options, templates, injectMedia) {
         const {
-            general: {resultOutputMode, compactGlossaries},
+            general: {resultOutputMode, compactGlossaries, compactTags},
             anki: {tags, checkForDuplicates, duplicateScope, kanji, terms, screenshot: {format, quality}},
             audio: {sources, customSourceUrl}
         } = options;
@@ -1462,6 +1463,7 @@ class Display extends EventDispatcher {
             duplicateScope,
             resultOutputMode,
             compactGlossaries,
+            compactTags,
             modeOptions
         });
     }
diff --git a/test/test-options-util.js b/test/test-options-util.js
index 6b04cb02..378338c9 100644
--- a/test/test-options-util.js
+++ b/test/test-options-util.js
@@ -608,6 +608,79 @@ ${update2}
 ${update4}
 ${update6}
 {{~> (lookup . "marker") ~}}`.trimStart()
+        },
+        // Definition tags update
+        {
+            old: `
+{{#*inline "glossary-single"}}
+    {{~#unless brief~}}
+        {{~#if definitionTags~}}<i>({{#each definitionTags}}{{name}}{{#unless @last}}, {{/unless}}{{/each}})</i> {{/if~}}
+        {{~#if only~}}({{#each only}}{{{.}}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
+    {{~/unless~}}
+{{/inline}}
+
+{{#*inline "glossary-single2"}}
+    {{~#unless brief~}}
+        {{~#if definitionTags~}}<i>({{#each definitionTags}}{{name}}{{#unless @last}}, {{/unless}}{{/each}})</i> {{/if~}}
+        {{~#if only~}}({{#each only}}{{{.}}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
+    {{~/unless~}}
+{{/inline}}
+
+{{#*inline "glossary"}}
+    {{~> glossary-single definition brief=brief compactGlossaries=compactGlossaries~}}
+    {{~> glossary-single definition brief=brief compactGlossaries=../compactGlossaries~}}
+{{/inline}}
+
+{{~> (lookup . "marker") ~}}
+`.trimStart(),
+
+            expected: `
+{{#*inline "glossary-single"}}
+    {{~#unless brief~}}
+        {{~#scope~}}
+            {{~#set "any" false}}{{/set~}}
+            {{~#if definitionTags~}}{{#each definitionTags~}}
+                {{~#if (op "||" (op "!" ../data.compactTags) (op "!" redundant))~}}
+                    {{~#if (get "any")}}, {{else}}<i>({{/if~}}
+                    {{name}}
+                    {{~#set "any" true}}{{/set~}}
+                {{~/if~}}
+            {{~/each~}}
+            {{~#if (get "any")}})</i> {{/if~}}
+            {{~/if~}}
+        {{~/scope~}}
+        {{~#if only~}}({{#each only}}{{{.}}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
+    {{~/unless~}}
+{{/inline}}
+
+{{#*inline "glossary-single2"}}
+    {{~#unless brief~}}
+        {{~#scope~}}
+            {{~#set "any" false}}{{/set~}}
+            {{~#if definitionTags~}}{{#each definitionTags~}}
+                {{~#if (op "||" (op "!" ../data.compactTags) (op "!" redundant))~}}
+                    {{~#if (get "any")}}, {{else}}<i>({{/if~}}
+                    {{name}}
+                    {{~#set "any" true}}{{/set~}}
+                {{~/if~}}
+            {{~/each~}}
+            {{~#if (get "any")}})</i> {{/if~}}
+            {{~/if~}}
+        {{~/scope~}}
+        {{~#if only~}}({{#each only}}{{{.}}}{{#unless @last}}, {{/unless}}{{/each}} only) {{/if~}}
+    {{~/unless~}}
+{{/inline}}
+
+{{#*inline "glossary"}}
+    {{~> glossary-single definition brief=brief compactGlossaries=compactGlossaries data=.~}}
+    {{~> glossary-single definition brief=brief compactGlossaries=../compactGlossaries data=../.~}}
+{{/inline}}
+
+${update2}
+${update4}
+${update6}
+{{~> (lookup . "marker") ~}}
+`.trimStart()
         }
     ];