Fix: fix #2343, properly fix "postfixDistinguished", also when marking as unknown

src/Logic/Tags/TagTypes.ts

@@ -25,6 +25,11 @@ export class TagTypes {
         return <any>and.and
     }
 
+    static uploadableAnd(and: And & UploadableTag): UploadableTag[] {
+        return <any>and.and
+    }
+
+
     static safeOr(or: Or & OptimizedTag): ((FlatTag | (And & OptimizedTag)) & OptimizedTag)[] {
         return <any>or.or
     }

src/Logic/Tags/TagUtils.ts

@@ -21,7 +21,7 @@ export class TagUtils {
         ["<=", (a, b) => a <= b],
         [">=", (a, b) => a >= b],
         ["<", (a, b) => a < b],
-        [">", (a, b) => a > b],
+        [">", (a, b) => a > b]
     ]
     public static modeDocumentation: Record<
         string,
@@ -48,7 +48,7 @@ export class TagUtils {
                 "### Removing a key\n" +
                 "\n" +
                 "If a key should be deleted in the OpenStreetMap-database, specify `key=` as well. This can be used e.g. to remove a\n" +
-                "fixme or value from another mapping if another field is filled out.",
+                "fixme or value from another mapping if another field is filled out."
         },
         "!=": {
             name: "strict not equals",
@@ -62,7 +62,7 @@ export class TagUtils {
                 "### If key is present\n" +
                 "\n" +
                 "This implies that, to check if a key is present, `key!=` can be used. This will only match if the key is present and not\n" +
-                "empty.",
+                "empty."
         },
         "~": {
             name: "Value matches regex",
@@ -73,12 +73,12 @@ export class TagUtils {
                 "The regex is put within braces as to prevent runaway values.\n" +
                 "\nUse `key~*` to indicate that any value is allowed. This is effectively the check that the attribute is present (defined _and_ not empty)." +
                 "\n" +
-                "Regexes will match the newline character with `.` too - the `s`-flag is enabled by default.",
+                "Regexes will match the newline character with `.` too - the `s`-flag is enabled by default."
         },
         "~i~": {
             name: "Value matches case-invariant regex",
             overpassSupport: true,
-            docs: "A tag can also be tested against a regex with `key~i~regex`, where the case of the value will be ignored. The regex is still matched against the _entire_ value",
+            docs: "A tag can also be tested against a regex with `key~i~regex`, where the case of the value will be ignored. The regex is still matched against the _entire_ value"
         },
         "!~": {
             name: "Value should _not_ match regex",
@@ -87,27 +87,27 @@ export class TagUtils {
                 "A tag can also be tested against a regex with `key!~regex`. This filter will match if the value does *not* match the regex. " +
                 "\n If the\n" +
                 "value is allowed to appear anywhere as substring, use `key~.*regex.*`.\n" +
-                "The regex is put within braces as to prevent runaway values.\n",
+                "The regex is put within braces as to prevent runaway values.\n"
         },
         "!~i~": {
             name: "Value does *not* match case-invariant regex",
             overpassSupport: true,
-            docs: "A tag can also be tested against a regex with `key~i~regex`, where the case of the value will be ignored. The regex is still matched against the _entire_ value. This filter returns true if the value does *not* match",
+            docs: "A tag can also be tested against a regex with `key~i~regex`, where the case of the value will be ignored. The regex is still matched against the _entire_ value. This filter returns true if the value does *not* match"
         },
         "~~": {
             name: "Key and value should match given regex",
             overpassSupport: true,
-            docs: "Both the `key` and `value` part of this specification are interpreted as regexes, both the key and value musth completely match their respective regexes",
+            docs: "Both the `key` and `value` part of this specification are interpreted as regexes, both the key and value musth completely match their respective regexes"
         },
         "~i~~": {
             name: "Key and value should match a given regex; value is case-invariant",
             overpassSupport: true,
-            docs: "Similar to ~~, except that the value is case-invariant",
+            docs: "Similar to ~~, except that the value is case-invariant"
         },
         "!~i~~": {
             name: "Key and value should match a given regex; value is case-invariant",
             overpassSupport: true,
-            docs: "Similar to !~~, except that the value is case-invariant",
+            docs: "Similar to !~~, except that the value is case-invariant"
         },
         ":=": {
             name: "Substitute `... {some_key} ...` and match `key`",
@@ -133,24 +133,24 @@ export class TagUtils {
                 "\n" +
                 "```json\n" +
                 "{\n" +
-                '    "mappings": [\n' +
+                "    \"mappings\": [\n" +
                 "        {\n" +
-                '            "if":"key:={some_other_key}",\n' +
+                "            \"if\":\"key:={some_other_key}\",\n" +
-                '            "then": "...",\n' +
+                "            \"then\": \"...\",\n" +
-                '            "hideInAnswer": "some_other_key="\n' +
+                "            \"hideInAnswer\": \"some_other_key=\"\n" +
                 "        }\n" +
                 "    ]\n" +
                 "}\n" +
                 "```\n" +
                 "\n" +
                 "One can use `key!:=prefix-{other_key}-postfix` as well, to match if `key` is _not_ the same\n" +
-                "as `prefix-{other_key}-postfix` (with `other_key` substituted by the value)",
+                "as `prefix-{other_key}-postfix` (with `other_key` substituted by the value)"
         },
         "!:=": {
             name: "Substitute `{some_key}` should not match `key`",
             overpassSupport: false,
-            docs: "See `:=`, except that this filter is inverted",
+            docs: "See `:=`, except that this filter is inverted"
-        },
+        }
     }
     private static keyCounts: { keys: any; tags: any } = key_counts
     public static readonly numberAndDateComparisonDocs =
@@ -175,10 +175,10 @@ export class TagUtils {
         "\n" +
         "```json\n" +
         "{\n" +
-        '  "osmTags": {\n' +
+        "  \"osmTags\": {\n" +
-        '    "or": [\n' +
+        "    \"or\": [\n" +
-        '      "amenity=school",\n' +
+        "      \"amenity=school\",\n" +
-        '      "amenity=kindergarten"\n' +
+        "      \"amenity=kindergarten\"\n" +
         "    ]\n" +
         "  }\n" +
         "}\n" +
@@ -194,7 +194,7 @@ export class TagUtils {
         "If the schema-files note a type [`TagConfigJson`](https://github.com/pietervdvn/MapComplete/blob/develop/src/Models/ThemeConfig/Json/TagConfigJson.ts), you can use one of these values.\n" +
         "\n" +
         "In some cases, not every type of tags-filter can be used. For example,  _rendering_ an option with a regex is\n" +
-        'fine (`"if": "brand~[Bb]randname", "then":" The brand is Brandname"`); but this regex can not be used to write a value\n' +
+        "fine (`\"if\": \"brand~[Bb]randname\", \"then\":\" The brand is Brandname\"`); but this regex can not be used to write a value\n" +
         "into the database. The theme loader will however refuse to work with such inconsistencies and notify you of this while\n" +
         "you are building your theme.\n" +
         "\n" +
@@ -205,18 +205,18 @@ export class TagUtils {
         "\n" +
         "```json\n" +
         "{\n" +
-        '  "and": [\n' +
+        "  \"and\": [\n" +
-        '    "key=value",\n' +
+        "    \"key=value\",\n" +
         "    {\n" +
-        '      "or": [\n' +
+        "      \"or\": [\n" +
-        '        "other_key=value",\n' +
+        "        \"other_key=value\",\n" +
-        '        "other_key=some_other_value"\n' +
+        "        \"other_key=some_other_value\"\n" +
         "      ]\n" +
         "    },\n" +
-        '    "key_which_should_be_missing=",\n' +
+        "    \"key_which_should_be_missing=\",\n" +
-        '    "key_which_should_have_a_value~*",\n' +
+        "    \"key_which_should_have_a_value~*\",\n" +
-        '    "key~.*some_regex_a*_b+_[a-z]?",\n' +
+        "    \"key~.*some_regex_a*_b+_[a-z]?\",\n" +
-        '    "height<1"\n' +
+        "    \"height<1\"\n" +
         "  ]\n" +
         "}\n" +
         "```\n" +
@@ -374,9 +374,9 @@ export class TagUtils {
      * TagUtils.FlattenMultiAnswer(([new Tag("x","y"), new Tag("a","b")])) // => new And([new Tag("x","y"), new Tag("a","b")])
      * TagUtils.FlattenMultiAnswer(([new Tag("x","")])) // => new And([new Tag("x","")])
      */
-    static FlattenMultiAnswer(tagsFilters: UploadableTag[]): And {
+    static FlattenMultiAnswer(tagsFilters: UploadableTag[]): UploadableTag[] {
         if (tagsFilters === undefined) {
-            return new And([])
+            return []
         }
 
         const keyValues = TagUtils.SplitKeys(tagsFilters)
@@ -386,7 +386,7 @@ export class TagUtils {
             values.sort()
             and.push(new Tag(key, values.join(";")))
         }
-        return new And(and)
+        return and
     }
 
     /**
@@ -985,10 +985,10 @@ export class TagUtils {
                 return ["", "## `" + mode + "` " + doc.name, "", doc.docs, "", ""].join("\n")
             }),
             "## " +
-                TagUtils.comparators.map((comparator) => "`" + comparator[0] + "`").join(" ") +
+            TagUtils.comparators.map((comparator) => "`" + comparator[0] + "`").join(" ") +
-                " Logical comparators",
+            " Logical comparators",
             TagUtils.numberAndDateComparisonDocs,
-            TagUtils.logicalOperator,
+            TagUtils.logicalOperator
         ].join("\n")
     }
 

src/Models/ThemeConfig/TagRenderingConfig.ts

@@ -734,7 +734,7 @@ export default class TagRenderingConfig {
         singleSelectedMapping: number,
         multiSelectedMapping: boolean[] | undefined,
         currentProperties: Record<string, string>
-    ): UploadableTag {
+    ): UploadableTag[] {
         if (typeof freeformValue === "string") {
             freeformValue = freeformValue?.trim()
         }
@@ -782,14 +782,14 @@ export default class TagRenderingConfig {
             const freeformOnly = { [this.freeform.key]: freeformValue }
             const matchingMapping = this.mappings?.find((m) => m.if.matchesProperties(freeformOnly))
             if (matchingMapping) {
-                return new And([matchingMapping.if, ...(matchingMapping.addExtraTags ?? [])])
+                return [matchingMapping.if, ...(matchingMapping.addExtraTags ?? [])]
             }
             // Either no mappings, or this is a radio-button selected freeform value
-            const tag = new And([
+            const tag = [
                 new Tag(this.freeform.key, freeformValue),
                 ...(this.freeform.addExtraTags ?? [])
-            ])
+            ]
-            const newProperties = tag.applyOn(currentProperties)
+            const newProperties = new And(tag).applyOn(currentProperties)
             if (this.invalidValues?.matchesProperties(newProperties)) {
                 return undefined
             }
@@ -816,14 +816,9 @@ export default class TagRenderingConfig {
                 )
             }
             const and = TagUtils.FlattenMultiAnswer([...selectedMappings, ...unselectedMappings])
-            if (and.and.length === 0) {
+            if (and.length === 0) {
                 return undefined
             }
-            console.log(
-                ">>> New properties",
-                TagUtils.asProperties(and, currentProperties),
-                this.invalidValues
-            )
             if (
                 this.invalidValues?.matchesProperties(TagUtils.asProperties(and, currentProperties))
             ) {
@@ -847,15 +842,15 @@ export default class TagRenderingConfig {
                 !someMappingIsShown ||
                 singleSelectedMapping === undefined)
         if (useFreeform) {
-            return new And([
+            return [
                 new Tag(this.freeform.key, freeformValue),
                 ...(this.freeform.addExtraTags ?? [])
-            ])
+            ]
         } else if (singleSelectedMapping !== undefined) {
-            return new And([
+            return [
                 this.mappings[singleSelectedMapping].if,
                 ...(this.mappings[singleSelectedMapping].addExtraTags ?? [])
-            ])
+            ]
         } else {
             console.error("TagRenderingConfig.ConstructSpecification has a weird fallback for", {
                 freeformValue,
@@ -864,7 +859,6 @@ export default class TagRenderingConfig {
                 currentProperties,
                 useFreeform
             })
-
             return undefined
         }
     }
@@ -998,7 +992,7 @@ export default class TagRenderingConfig {
      * The keys that should be erased if one has to revert to 'unknown'.
      * Might give undefined if setting to unknown is not possible
      */
-    public removeToSetUnknown(
+    private removeToSetUnknown(
         partOfLayer: LayerConfig,
         currentTags: Record<string, string>
     ): string[] | undefined {
@@ -1042,6 +1036,23 @@ export default class TagRenderingConfig {
 
         return Array.from(toDelete)
     }
+
+    /**
+     * Gives all the tags that should be applied to "reset" the freeform key to an "unknown" state
+     */
+    public markUnknown(layer: LayerConfig, currentProperties: Record<string, string>): UploadableTag[] {
+        if (this.freeform?.postfixDistinguished) {
+            const allValues = currentProperties[this.freeform.key].split(";").filter(
+                part => part.split("/")[1]?.trim() !== this.freeform.postfixDistinguished
+            )
+            return [new Tag(this.freeform.key, allValues.join(";"))]
+        }
+
+        const keys = this.removeToSetUnknown(layer, currentProperties)
+
+
+        return keys?.map(k => new Tag(k, ""))
+    }
 }
 
 export class TagRenderingConfigUtils {

src/UI/Popup/TagHint.svelte

@@ -3,29 +3,30 @@
   import FromHtml from "../Base/FromHtml.svelte"
   import { Translation } from "../i18n/Translation"
   import Tr from "../Base/Tr.svelte"
-  import type { SpecialVisualizationState } from "../SpecialVisualization"
   import Translations from "../i18n/Translations"
 
   /**
    * A 'TagHint' will show the given tags in a human readable form.
    * Depending on the options, it'll link through to the wiki or might be completely hidden
    */
-  export let tags: TagsFilter
+  export let tags: TagsFilter[]
-  export let currentProperties: Record<string, string | any> = {}
+  export let currentProperties: Record<string, string> = {}
-  /**
-   * If given, this function will be called to embed the given tags hint into this translation
-   */
-  export let embedIn: ((string: string) => Translation) | undefined = undefined
-  let tagsExplanation = ""
-  $: tagsExplanation = tags?.asHumanString(true, false, currentProperties)
 </script>
 
-<div class="break-words" style="word-break: break-word">
+{#if tags?.length > 0}
-  {#if tags === undefined}
+  {#each tags as tag}
-    <slot name="no-tags"><Tr cls="subtle" t={Translations.t.general.noTagsSelected} /></slot>
+    <div class="break-words" style="word-break: break-word">
-  {:else if embedIn === undefined}
+      {#if tag["value"] === ""}
-    <FromHtml src={tagsExplanation} />
+        <del>
-  {:else}
+          {tag["key"]}
-    <Tr t={embedIn(tagsExplanation)} />
+        </del>
-  {/if}
+      {:else}
-</div>
+        <FromHtml src={tag.asHumanString(true, false, currentProperties)} />
+      {/if}
+    </div>
+  {/each}
+{:else}
+  <slot name="no-tags">
+    <Tr cls="subtle" t={Translations.t.general.noTagsSelected} />
+  </slot>
+{/if}

src/UI/Popup/TagRendering/TagRenderingQuestion.svelte

@@ -1,5 +1,5 @@
 <script lang="ts">
-  import { ImmutableStore, UIEventSource } from "../../../Logic/UIEventSource"
+  import { ImmutableStore, Store, UIEventSource } from "../../../Logic/UIEventSource"
   import type { SpecialVisualizationState } from "../../SpecialVisualization"
   import Tr from "../../Base/Tr.svelte"
   import type { Feature } from "geojson"
@@ -31,7 +31,9 @@
   import { get } from "svelte/store"
   import Markdown from "../../Base/Markdown.svelte"
   import { Utils } from "../../../Utils"
+  import { TagTypes } from "../../../Logic/Tags/TagTypes"
   import type { UploadableTag } from "../../../Logic/Tags/TagTypes"
+
   import Popup from "../../Base/Popup.svelte"
   import If from "../../Base/If.svelte"
   import DotMenu from "../../Base/DotMenu.svelte"
@@ -43,7 +45,7 @@
   export let selectedElement: Feature
   export let state: SpecialVisualizationState
   export let layer: LayerConfig | undefined
-  export let selectedTags: UploadableTag = undefined
+  export let selectedTags: UploadableTag[] = undefined
   export let extraTags: UIEventSource<Record<string, string>> = new UIEventSource({})
 
   export let clss = "interactive border-interactive"
@@ -65,9 +67,9 @@
   let checkedMappings: boolean[]
 
   /**
-   * IF set: we can remove the current answer by deleting all those keys
+   * The tags to apply to mark this answer as "unknown"
    */
-  let settableKeys = tags.mapD((tags) => config.removeToSetUnknown(layer, tags))
+  let onMarkUnknown: Store<UploadableTag[] | undefined> = tags.mapD((tags) => config.markUnknown(layer, tags))
   let unknownModal = new UIEventSource(false)
 
   let searchTerm: UIEventSource<string> = new UIEventSource("")
@@ -118,7 +120,7 @@
             seenFreeforms.push(newProps[confg.freeform.key])
           }
           return matches
-        }),
+        })
       ]
 
       if (tgs !== undefined && confg.freeform) {
@@ -211,7 +213,7 @@
       !config.freeform.postfixDistinguished &&
       $tags[config.freeform.key] // We need to have a current value in order to delete it
     ) {
-      selectedTags = new Tag(config.freeform.key, "")
+      selectedTags = [new Tag(config.freeform.key, "")]
     } else {
       try {
         selectedTags = config?.constructChangeSpecification(
@@ -227,7 +229,7 @@
               freeform: $freeformInput,
               selectedMapping,
               checkedMappings,
-              currentTags: tags.data,
+              currentTags: tags.data
             },
             " --> ",
             selectedTags
@@ -246,10 +248,10 @@
       // Check the type of selectedTags
       if (selectedTags instanceof Tag) {
         // Re-define selectedTags as an And
-        selectedTags = new And([selectedTags, ...extraTagsArray])
+        selectedTags = [selectedTags, ...extraTagsArray]
       } else if (selectedTags instanceof And) {
         // Add the extraTags to the existing And
-        selectedTags = new And([...selectedTags.and, ...extraTagsArray])
+        selectedTags = [...TagTypes.uploadableAnd(selectedTags), ...extraTagsArray]
       } else {
         console.error(
           "selectedTags is not of type Tag or And, it is a " + JSON.stringify(selectedTags)
@@ -258,17 +260,18 @@
     }
   }
 
-  function onSave(_ = undefined) {
+  function onSave() {
     if (selectedTags === undefined) {
       return
     }
 
+    const selectedTagsJoined = new And(selectedTags)
     if (layer === undefined || (layer?.source === null && layer.id !== "favourite")) {
       /**
        * This is a special, privileged layer.
        * We simply apply the tags onto the records
        */
-      const kv = selectedTags.asChange(tags.data)
+      const kv = selectedTagsJoined.asChange(tags.data)
       for (const { k, v } of kv) {
         if (v === undefined) {
           // Note: we _only_ delete if it is undefined. We _leave_ the empty string and assign it, so that data consumers get correct information
@@ -279,13 +282,13 @@
         feedback.setData(undefined)
       }
       tags.ping()
-      dispatch("saved", { config, applied: selectedTags })
+      dispatch("saved", { config, applied: selectedTagsJoined })
       return
     }
-    dispatch("saved", { config, applied: selectedTags })
+    dispatch("saved", { config, applied: selectedTagsJoined })
-    const change = new ChangeTagAction(tags.data.id, selectedTags, tags.data, {
+    const change = new ChangeTagAction(tags.data.id, selectedTagsJoined, tags.data, {
       theme: tags.data["_orig_theme"] ?? state.theme?.id,
-      changeType: "answer",
+      changeType: "answer"
     })
     freeformInput.set(undefined)
     selectedMapping = undefined
@@ -326,10 +329,10 @@
   }
 
   function clearAnswer() {
-    const tagsToSet = settableKeys.data.map((k) => new Tag(k, ""))
+    const tagsToSet: UploadableTag[] = onMarkUnknown.data
     const change = new ChangeTagAction(tags.data.id, new And(tagsToSet), tags.data, {
       theme: tags.data["_orig_theme"] ?? state.theme.id,
-      changeType: "answer",
+      changeType: "answer"
     })
     freeformInput.set(undefined)
     selectedMapping = undefined
@@ -576,13 +579,7 @@
             >
               <div class="subtle">
                 <Tr t={Translations.t.unknown.removedKeys} />
-                {#each $settableKeys as key}
+                <TagHint tags={$onMarkUnknown}></TagHint>
-                  <code>
-                    <del>
-                      {key}
-                    </del>
-                  </code>
-                {/each}
               </div>
             </If>
             <div class="flex w-full justify-end" slot="footer">
@@ -602,7 +599,7 @@
           </Popup>
 
           <div class="sticky bottom-0 flex flex-wrap justify-between" style="z-index: 11">
-            {#if $settableKeys && $isKnown && !matchesEmpty}
+            {#if $onMarkUnknown?.length > 0 && $isKnown && !matchesEmpty}
               <button class="as-link small text-sm" on:click={() => unknownModal.set(true)}>
                 <Tr t={Translations.t.unknown.markUnknown} />
               </button>
@@ -614,8 +611,13 @@
               <!-- TagRenderingQuestion-buttons -->
               <slot name="cancel" />
               <slot name="save-button" {selectedTags}>
-                {#if config.freeform?.key && !checkedMappings?.some((m) => m) && !$freeformInput && !$freeformInputUnvalidated && $tags[config.freeform.key]
+
-                && (!config.freeform.postfixDistinguished)}
+                <!-- Save-button / delete button -->
+                {#if config.freeform?.key &&
+                !checkedMappings?.some((m) => m) &&
+                !$freeformInput && !$freeformInputUnvalidated
+                && $tags[config.freeform.key]
+                && $isKnown}
                   <button
                     class="primary flex"
                     on:click|stopPropagation|preventDefault={() => onSave()}
@@ -637,9 +639,10 @@
               </slot>
             </div>
           </div>
+          <!-- Taghint + debug info -->
           {#if UserRelatedState.SHOW_TAGS_VALUES.indexOf($showTags) >= 0 || ($showTags === "" && numberOfCs >= Constants.userJourney.tagsVisibleAt) || $featureSwitchIsTesting || $featureSwitchIsDebugging}
             <span class="flex flex-wrap justify-between">
-              <TagHint {state} tags={selectedTags} currentProperties={$tags} />
+              <TagHint tags={selectedTags} currentProperties={$tags} />
               <span class="flex flex-wrap">
                 {#if $featureSwitchIsTesting}
                   <div class="alert" style="padding: 0; margin: 0; margin-right:  0.5rem">
@@ -647,9 +650,9 @@
                   </div>
                 {/if}
                 {#if $featureSwitchIsTesting || $featureSwitchIsDebugging}
-                  <a class="small" on:click={() => console.log("Configuration is ", config)}>
+                  <button class="small as-link" on:click={() => console.log("Configuration is ", config)}>
                     {config.id}
-                  </a>
+                  </button>
                 {/if}
               </span>
             </span>