From 2286ec964f4834651ceeef0e3067faacc5b5837f Mon Sep 17 00:00:00 2001
From: Pieter Vander Vennet <pietervdvn@posteo.net>
Date: Sun, 9 Mar 2025 23:23:37 +0100
Subject: [PATCH] Fix: fix #2343, properly fix "postfixDistinguished", also
when marking as unknown
---
src/Logic/Tags/TagTypes.ts | 5 ++
src/Logic/Tags/TagUtils.ts | 74 +++++++++----------
src/Models/ThemeConfig/TagRenderingConfig.ts | 45 ++++++-----
src/UI/Popup/TagHint.svelte | 37 +++++-----
.../TagRendering/TagRenderingQuestion.svelte | 63 ++++++++--------
5 files changed, 122 insertions(+), 102 deletions(-)
diff --git a/src/Logic/Tags/TagTypes.ts b/src/Logic/Tags/TagTypes.ts
index c46d13126..17e37d96e 100644
--- a/src/Logic/Tags/TagTypes.ts
+++ b/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
}
diff --git a/src/Logic/Tags/TagUtils.ts b/src/Logic/Tags/TagUtils.ts
index c3933cdca..aa92c24bd 100644
--- a/src/Logic/Tags/TagUtils.ts
+++ b/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' +
- ' "then": "...",\n' +
- ' "hideInAnswer": "some_other_key="\n' +
+ " \"if\":\"key:={some_other_key}\",\n" +
+ " \"then\": \"...\",\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' +
- ' "or": [\n' +
- ' "amenity=school",\n' +
- ' "amenity=kindergarten"\n' +
+ " \"osmTags\": {\n" +
+ " \"or\": [\n" +
+ " \"amenity=school\",\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' +
- ' "key=value",\n' +
+ " \"and\": [\n" +
+ " \"key=value\",\n" +
" {\n" +
- ' "or": [\n' +
- ' "other_key=value",\n' +
- ' "other_key=some_other_value"\n' +
+ " \"or\": [\n" +
+ " \"other_key=value\",\n" +
+ " \"other_key=some_other_value\"\n" +
" ]\n" +
" },\n" +
- ' "key_which_should_be_missing=",\n' +
- ' "key_which_should_have_a_value~*",\n' +
- ' "key~.*some_regex_a*_b+_[a-z]?",\n' +
- ' "height<1"\n' +
+ " \"key_which_should_be_missing=\",\n" +
+ " \"key_which_should_have_a_value~*\",\n" +
+ " \"key~.*some_regex_a*_b+_[a-z]?\",\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(" ") +
- " Logical comparators",
+ TagUtils.comparators.map((comparator) => "`" + comparator[0] + "`").join(" ") +
+ " Logical comparators",
TagUtils.numberAndDateComparisonDocs,
- TagUtils.logicalOperator,
+ TagUtils.logicalOperator
].join("\n")
}
diff --git a/src/Models/ThemeConfig/TagRenderingConfig.ts b/src/Models/ThemeConfig/TagRenderingConfig.ts
index c8ab20a99..4db5a4f05 100644
--- a/src/Models/ThemeConfig/TagRenderingConfig.ts
+++ b/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 {
diff --git a/src/UI/Popup/TagHint.svelte b/src/UI/Popup/TagHint.svelte
index 42e7ba733..49549507c 100644
--- a/src/UI/Popup/TagHint.svelte
+++ b/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 currentProperties: Record<string, string | any> = {}
- /**
- * 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)
+ export let tags: TagsFilter[]
+ export let currentProperties: Record<string, string> = {}
</script>
-<div class="break-words" style="word-break: break-word">
- {#if tags === undefined}
- <slot name="no-tags"><Tr cls="subtle" t={Translations.t.general.noTagsSelected} /></slot>
- {:else if embedIn === undefined}
- <FromHtml src={tagsExplanation} />
- {:else}
- <Tr t={embedIn(tagsExplanation)} />
- {/if}
-</div>
+{#if tags?.length > 0}
+ {#each tags as tag}
+ <div class="break-words" style="word-break: break-word">
+ {#if tag["value"] === ""}
+ <del>
+ {tag["key"]}
+ </del>
+ {:else}
+ <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}
diff --git a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte b/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte
index 3b354959e..f78dc6916 100644
--- a/src/UI/Popup/TagRendering/TagRenderingQuestion.svelte
+++ b/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 })
- const change = new ChangeTagAction(tags.data.id, selectedTags, tags.data, {
+ dispatch("saved", { config, applied: selectedTagsJoined })
+ 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}
- <code>
- <del>
- {key}
- </del>
- </code>
- {/each}
+ <TagHint tags={$onMarkUnknown}></TagHint>
</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>