From 2c3c110624e6128c17ba9c31f38a763c3591758e Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Sun, 14 Mar 2021 01:40:35 +0100 Subject: [PATCH] Add more docs, add more checks, add more tests --- Customizations/AllKnownLayers.ts | 4 +- Customizations/AllKnownLayouts.ts | 4 +- Customizations/JSON/TagRenderingConfig.ts | 31 +++++++++- Customizations/JSON/TagRenderingConfigJson.ts | 60 +++++++++++++++++-- Docs/Tags_format.md | 37 ++++++++++++ Logic/Tags.ts | 11 +++- test/Tag.spec.ts | 28 ++++++++- 7 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 Docs/Tags_format.md diff --git a/Customizations/AllKnownLayers.ts b/Customizations/AllKnownLayers.ts index c526eb3b5..ff143e415 100644 --- a/Customizations/AllKnownLayers.ts +++ b/Customizations/AllKnownLayers.ts @@ -23,6 +23,7 @@ import * as benches from "../assets/layers/benches/benches.json" import * as benches_at_pt from "../assets/layers/benches/benches_at_pt.json" import * as picnic_tables from "../assets/layers/benches/picnic_tables.json" import * as play_forest from "../assets/layers/play_forest/play_forest.json" +import * as playground from "../assets/layers/playground/playground.json" import LayerConfig from "./JSON/LayerConfig"; import {LayerConfigJson} from "./JSON/LayerConfigJson"; @@ -54,7 +55,8 @@ export default class AllKnownLayers { benches, benches_at_pt, picnic_tables, - play_forest + play_forest, + playground ]; // Must be below the list... diff --git a/Customizations/AllKnownLayouts.ts b/Customizations/AllKnownLayouts.ts index fb8f7cd72..2179fe090 100644 --- a/Customizations/AllKnownLayouts.ts +++ b/Customizations/AllKnownLayouts.ts @@ -22,6 +22,7 @@ import * as personal from "../assets/themes/personalLayout/personalLayout.json" import * as playgrounds from "../assets/themes/playgrounds/playgrounds.json" import * as bicycle_lib from "../assets/themes/bicycle_library/bicycle_library.json" import * as play_forests from "../assets/themes/play_forests/play_forests.json" +import * as speelplekken from "../assets/themes/speelplekken/speelplekken.json" import LayerConfig from "./JSON/LayerConfig"; import LayoutConfig from "./JSON/LayoutConfig"; import AllKnownLayers from "./AllKnownLayers"; @@ -71,7 +72,8 @@ export class AllKnownLayouts { new LayoutConfig(climbing), new LayoutConfig(playgrounds), new LayoutConfig(trees), - new LayoutConfig(play_forests) + new LayoutConfig(play_forests) , + new LayoutConfig(speelplekken) ]; diff --git a/Customizations/JSON/TagRenderingConfig.ts b/Customizations/JSON/TagRenderingConfig.ts index 20ee771d5..0c54c34ec 100644 --- a/Customizations/JSON/TagRenderingConfig.ts +++ b/Customizations/JSON/TagRenderingConfig.ts @@ -82,8 +82,11 @@ export default class TagRenderingConfig { this.multiAnswer = json.multiAnswer ?? false if (json.mappings) { + + this.mappings = json.mappings.map((mapping, i) => { + if (mapping.then === undefined) { throw `${context}.mapping[${i}]: Invalid mapping: if without body` } @@ -104,7 +107,7 @@ export default class TagRenderingConfig { hideInAnswer: hideInAnswer }; if (this.question) { - if (hideInAnswer !== true && !mp.if.isUsableAsAnswer()) { + if (hideInAnswer !== true && mp.if !== undefined && !mp.if.isUsableAsAnswer()) { throw `${context}.mapping[${i}].if: This value cannot be used to answer a question, probably because it contains a regex or an OR. Either change it or set 'hideInAnswer'` } @@ -128,6 +131,32 @@ export default class TagRenderingConfig { if(this.render && this.question && this.freeform === undefined){ throw `${context}: Detected a tagrendering which takes input without freeform key in ${context}` } + + if(!json.multiAnswer && this.mappings !== undefined && this.question !== undefined){ + let keys = [] + for (let i = 0; i < this.mappings.length; i++){ + const mapping = this.mappings[i]; + if(mapping.if === undefined){ + throw `${context}.mappings[${i}].if is undefined` + } + keys.push(...mapping.if.usedKeys()) + } + keys = Utils.Dedup(keys) + for (let i = 0; i < this.mappings.length; i++){ + const mapping = this.mappings[i]; + if(mapping.hideInAnswer){ + continue + } + + const usedKeys = mapping.if.usedKeys(); + for (const expectedKey of keys) { + if(usedKeys.indexOf(expectedKey) < 0){ + const msg = `${context}.mappings[${i}]: This mapping only defines values for ${usedKeys.join(', ')}, but it should also give a value for ${expectedKey}` + console.warn(msg) + } + } + } + } if (this.question !== undefined && json.multiAnswer) { if ((this.mappings?.length ?? 0) === 0) { diff --git a/Customizations/JSON/TagRenderingConfigJson.ts b/Customizations/JSON/TagRenderingConfigJson.ts index a440b96fd..30bbf5c78 100644 --- a/Customizations/JSON/TagRenderingConfigJson.ts +++ b/Customizations/JSON/TagRenderingConfigJson.ts @@ -49,14 +49,64 @@ export interface TagRenderingConfigJson { * Allows fixed-tag inputs, shown either as radiobuttons or as checkboxes */ mappings?: { + /** + * If this condition is met, then the text under `then` will be shown. + * If no value matches, and the user selects this mapping as an option, then these tags will be uploaded to OSM. + */ if: AndOrTagConfigJson | string, /** - * Only applicable if 'multiAnswer' is set. - * This tag is applied if the respective checkbox is unset + * If the condition `if` is met, the text `then` will be rendered. + * If not known yet, the user will be presented with `then` as an option */ - ifnot?: AndOrTagConfigJson | string, - then: string | any - hideInAnswer?: boolean + then: string | any, + /** + * In some cases, multiple taggings exist (e.g. a default assumption, or a commonly mapped abbreviation and a fully written variation). + * + * In the latter case, a correct text should be shown, but only a single, canonical tagging should be selectable by the user. + * In this case, one of the mappings can be hiden by setting this flag. + * + * To demonstrate an example making a default assumption: + * + * mappings: [ + * { + * if: "access=", -- no access tag present, we assume accessible + * then: "Accessible to the general public", + * hideInAnswer: true + * }, + * { + * if: "access=yes", + * then: "Accessible to the general public", -- the user selected this, we add that to OSM + * }, + * { + * if: "access=no", + * then: "Not accessible to the public" + * } + * ] + * + * + * For example, for an operator, we have `operator=Agentschap Natuur en Bos`, which is often abbreviated to `operator=ANB`. + * Then, we would add two mappings: + * { + * if: "operator=Agentschap Natuur en Bos" -- the non-abbreviated version which should be uploaded + * then: "Maintained by Agentschap Natuur en Bos" + * }, + * { + * if: "operator=ANB", -- we don't want to upload abbreviations + * then: "Maintained by Agentschap Natuur en Bos" + * hideInAnswer: true + * } + */ + hideInAnswer?: boolean, + /** + * Only applicable if 'multiAnswer' is set. + * This is for situations such as: + * `accepts:coins=no` where one can select all the possible payment methods. However, we want to make explicit that some options _were not_ selected. + * This can be done with `ifnot` + * Note that we can not explicitly render this negative case to the user, we cannot show `does _not_ accept coins`. + * If this is important to your usecase, consider using multiple radiobutton-fields without `multiAnswer` + */ + ifnot?: AndOrTagConfigJson | string + }[] /** diff --git a/Docs/Tags_format.md b/Docs/Tags_format.md new file mode 100644 index 000000000..4ea7c9c96 --- /dev/null +++ b/Docs/Tags_format.md @@ -0,0 +1,37 @@ + Tags format +============= + +When creating the `json` file describing your layer or theme, you'll have to add a few tags to describe what you want. This document gives an overview of what every expression means and how it behaves in edge cases. + +Strict equality +--------------- + +Strict equality is denoted by `key=value`. This key matches __only if__ the keypair is present exactly as stated. + +**Only normal tags (eventually in an `and`) can be used in places where they are uploaded**. Normal tags are used in the `mappings` of a [TagRendering] (unless `hideInAnswer` is specified), they are used in `addExtraTags` of [Freeform] and are used in the `tags`-list of a preset. + +If a different kind of tag specification is given, your theme will fail to parse. + +### If key is not present + +If you want to check if a key is not present, use `key=` (pronounce as *key is empty*). A tag collection will match this if `key` is missing or if `key` is a literal empty value. + +### Removing a key + +If a key should be deleted in the OpenStreetMap-database, specify `key=` as well. This can be used e.g. to remove a fixme or value from another mapping if another field is filled out. + +Strict not equals +----------------- + +To check if a key does _not_ equal a certain value, use `key!=value`. This is converted behind the scenes to `key!~^value$` + +### If key is present + +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 empty. + +Regex equals +------------ + +A tag can also be tested against a regex with `key~regex`. Note that this regex __must match__ the entire value. If the value is allowed to appear anywhere as substring, use `key~.*regex.*` + +Equivalently, `key!~regex` can be used if you _don't_ want to match the regex in order to appear. diff --git a/Logic/Tags.ts b/Logic/Tags.ts index 45e03b63c..ec3d733d6 100644 --- a/Logic/Tags.ts +++ b/Logic/Tags.ts @@ -1,5 +1,4 @@ import {Utils} from "../Utils"; -import {type} from "os"; export abstract class TagsFilter { abstract matches(tags: { k: string, v: string }[]): boolean @@ -26,12 +25,14 @@ export class RegexTag extends TagsFilter { private readonly key: RegExp | string; private readonly value: RegExp | string; private readonly invert: boolean; + private readonly matchesEmpty: boolean constructor(key: string | RegExp, value: RegExp | string, invert: boolean = false) { super(); this.key = key; this.value = value; this.invert = invert; + this.matchesEmpty = RegexTag.doesMatch("", this.value); } private static doesMatch(fromTag: string, possibleRegex: string | RegExp): boolean { @@ -65,6 +66,10 @@ export class RegexTag extends TagsFilter { return RegexTag.doesMatch(tag.v, this.value) != this.invert; } } + if(this.matchesEmpty){ + // The value is 'empty' + return !this.invert; + } // The matching key was not found return this.invert; } @@ -244,7 +249,7 @@ export class Or extends TagsFilter { } usedKeys(): string[] { - return [].concat(this.or.map(subkeys => subkeys.usedKeys())); + return [].concat(...this.or.map(subkeys => subkeys.usedKeys())); } } @@ -363,7 +368,7 @@ export class And extends TagsFilter { } usedKeys(): string[] { - return [].concat(this.and.map(subkeys => subkeys.usedKeys())); + return [].concat(...this.and.map(subkeys => subkeys.usedKeys())); } } diff --git a/test/Tag.spec.ts b/test/Tag.spec.ts index edc2adc04..bb8286116 100644 --- a/test/Tag.spec.ts +++ b/test/Tag.spec.ts @@ -27,6 +27,26 @@ new T("Tags", [ const tag = FromJSON.Tag("key=value") as Tag; equal(tag.key, "key"); equal(tag.value, "value"); + equal(tag.matches([{k:"key",v:"value"}]), true) + equal(tag.matches([{k:"key",v:"z"}]), false) + equal(tag.matches([{k:"key",v:""}]), false) + equal(tag.matches([{k:"other_key",v:""}]), false) + equal(tag.matches([{k:"other_key",v:"value"}]), false) + + const isEmpty = FromJSON.Tag("key=") as Tag; + equal(isEmpty.matches([{k:"key",v:"value"}]), false) + equal(isEmpty.matches([{k:"key",v:""}]), true) + equal(isEmpty.matches([{k:"other_key",v:""}]), true) + equal(isEmpty.matches([{k:"other_key",v:"value"}]), true) + + const isNotEmpty = FromJSON.Tag("key!="); + equal(isNotEmpty.matches([{k:"key",v:"value"}]), true) + equal(isNotEmpty.matches([{k:"key",v:"other_value"}]), true) + equal(isNotEmpty.matches([{k:"key",v:""}]), false) + equal(isNotEmpty.matches([{k:"other_key",v:""}]), false) + equal(isNotEmpty.matches([{k:"other_key",v:"value"}]), false) + + const and = FromJSON.Tag({"and": ["key=value", "x=y"]}) as And; equal((and.and[0] as Tag).key, "key"); @@ -37,10 +57,8 @@ new T("Tags", [ equal(notReg.matches([{k:"x",v:"y"}]), false) equal(notReg.matches([{k:"x",v:"z"}]), true) equal(notReg.matches([{k:"x",v:""}]), true) - equal(notReg.matches([]), true) - const noMatch = FromJSON.Tag("key!=value") as Tag; equal(noMatch.matches([{k:"key",v:"value"}]), false) equal(noMatch.matches([{k:"key",v:"otherValue"}]), true) @@ -53,6 +71,12 @@ new T("Tags", [ equal(multiMatch.matches([{k:"vending",v:"something;bicycle_tube"}]), true) equal(multiMatch.matches([{k:"vending",v:"bicycle_tube;something"}]), true) equal(multiMatch.matches([{k:"vending",v:"xyz;bicycle_tube;something"}]), true) + + const nameStartsWith = FromJSON.Tag("name~[sS]peelbos.*") + equal(nameStartsWith.matches([{k:"name",v: "Speelbos Sint-Anna"}]), true) + equal(nameStartsWith.matches([{k:"name",v: "speelbos Sint-Anna"}]), true) + equal(nameStartsWith.matches([{k:"name",v: "Sint-Anna"}]), false) + equal(nameStartsWith.matches([{k:"name",v: ""}]), false) })], ["Is equivalent test", (() => {