Bastian-Greshake-Tzovaras/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Themes: add validation check if a mapping does not erase another mapping completely

Browse source
This commit is contained in:
Pieter Vander Vennet 2023-11-09 15:42:15 +01:00
parent 7d43bb5983
commit 556f6d0b93
43 changed files with 5015 additions and 4778 deletions
Download patch file Download diff file Expand all files Collapse all files

101
src/Models/ThemeConfig/Conversion/Validation.ts
View file

@ -357,7 +357,7 @@ export class PrevalidateTheme extends Fuse<LayoutConfigJson> {
export class DetectConflictingAddExtraTags extends DesugaringStep<TagRenderingConfigJson> {
constructor() {
super(
"The `if`-part in a mapping might set some keys. Those key are not allowed to be set in the `addExtraTags`, as this might result in conflicting values",
"The `if`-part in a mapping might set some keys. Those keys are not allowed to be set in the `addExtraTags`, as this might result in conflicting values",
[],
"DetectConflictingAddExtraTags"
)
@ -399,6 +399,100 @@ export class DetectConflictingAddExtraTags extends DesugaringStep<TagRenderingCo
}
}
export class DetectNonErasedKeysInMappings extends DesugaringStep<QuestionableTagRenderingConfigJson> {
constructor() {
super(
"A tagRendering might set a freeform key (e.g. `name` and have an option that _should_ erase this name, e.g. `noname=yes`). Under normal circumstances, every mapping/freeform should affect all touched keys",
[],
"DetectNonErasedKeysInMappings"
)
}
convert(
json: QuestionableTagRenderingConfigJson,
context: ConversionContext
): QuestionableTagRenderingConfigJson {
if (json.multiAnswer) {
// No need to check this here, this has its own validation
return json
}
if (!json.question) {
// No need to check the writable tags, as this cannot write
return json
}
function addAll(keys: { forEach: (f: (s: string) => void) => void }, addTo: Set<string>) {
keys?.forEach((k) => addTo.add(k))
}
const freeformKeys: Set<string> = new Set()
if (json.freeform) {
freeformKeys.add(json.freeform.key)
for (const tag of json.freeform.addExtraTags ?? []) {
const tagParsed = TagUtils.Tag(tag)
addAll(tagParsed.usedKeys(), freeformKeys)
}
}
const mappingKeys: Set<string>[] = []
for (const mapping of json.mappings ?? []) {
if (mapping.hideInAnswer === true) {
mappingKeys.push(undefined)
continue
}
const thisMappingKeys: Set<string> = new Set<string>()
addAll(TagUtils.Tag(mapping.if).usedKeys(), thisMappingKeys)
for (const tag of mapping.addExtraTags ?? []) {
addAll(TagUtils.Tag(tag).usedKeys(), thisMappingKeys)
}
mappingKeys.push(thisMappingKeys)
}
const neededKeys = new Set<string>()
addAll(freeformKeys, neededKeys)
for (const mappingKey of mappingKeys) {
addAll(mappingKey, neededKeys)
}
neededKeys.delete("fixme") // fixme gets a free pass
if (json.freeform) {
for (const neededKey of neededKeys) {
if (!freeformKeys.has(neededKey)) {
context
.enters("freeform")
.warn(
"The freeform block does not modify the key `" +
neededKey +
"` which is set in a mapping. Use `addExtraTags` to overwrite it"
)
}
}
}
for (let i = 0; i < json.mappings?.length; i++) {
const mapping = json.mappings[i]
if (mapping.hideInAnswer === true) {
continue
}
const keys = mappingKeys[i]
for (const neededKey of neededKeys) {
if (!keys.has(neededKey)) {
context
.enters("mappings", i)
.warn(
"This mapping does not modify the key `" +
neededKey +
"` which is set in a mapping or by the freeform block. Use `addExtraTags` to overwrite it"
)
}
}
}
return json
}
}
export class DetectShadowedMappings extends DesugaringStep<TagRenderingConfigJson> {
private readonly _calculatedTagNames: string[]
@ -874,6 +968,7 @@ export class ValidateTagRenderings extends Fuse<TagRenderingConfigJson> {
"Various validation on tagRenderingConfigs",
new DetectShadowedMappings(layerConfig),
new DetectConflictingAddExtraTags(),
new DetectNonErasedKeysInMappings(),
new DetectMappingsWithImages(doesImageExist),
new On("render", new ValidatePossibleLinks()),
new On("question", new ValidatePossibleLinks()),
@ -1195,6 +1290,10 @@ export class PrevalidateLayer extends DesugaringStep<LayerConfigJson> {
const baseTags = TagUtils.Tag(json.source["osmTags"])
for (let i = 0; i < json.presets.length; i++) {
const preset = json.presets[i]
if (!preset) {
context.enters("presets", i).err("This preset is undefined")
continue
}
if (!preset.tags) {
context.enters("presets", i, "tags").err("No tags defined for this preset")
continue

6
src/Models/ThemeConfig/Json/LayoutConfigJson.ts
View file

@ -116,7 +116,7 @@ export interface LayoutConfigJson {
* type: float
* group: start_location
*/
startZoom: number
startZoom?: number
/**
* question: At what start latitude should this theme open?
* Default location and zoom to start.
@ -125,7 +125,7 @@ export interface LayoutConfigJson {
* type: float
* group: start_location
*/
startLat: number
startLat?: number
/**
* question: At what start longitude should this theme open?
* Default location and zoom to start.
@ -134,7 +134,7 @@ export interface LayoutConfigJson {
* type: float
* group: start_location
*/
startLon: number
startLon?: number
/**
* The id of the default background. BY default: vanilla OSM
*/

51
src/Models/ThemeConfig/Json/PointRenderingConfigJson.ts
View file

@ -50,6 +50,7 @@ export default interface PointRenderingConfigJson {
* They will be added as a 25% height icon at the bottom right of the icon, with all the badges in a flex layout.
*
* Note: strings are interpreted as icons, so layering and substituting is supported. You can use `circle:white;./my_icon.svg` to add a background circle
* group: hidden
*/
iconBadges?: {
if: TagConfigJson
@ -95,6 +96,29 @@ export default interface PointRenderingConfigJson {
*/
label?: string | TagRenderingConfigJson
/**
* question: What CSS should be applied to the label?
* You can set the css-properties here, e.g. `background: red; font-size: 12px; `
* inline: Apply CSS-style <b>{value}</b> to the label
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS-labels to the label
* group: expert
*/
labelCss?: TagRenderingConfigJson | string
/**
* question: Which CSS-classes should be applied to the label?
*
* The classes should be separated by a space (` `)
* You can use most Tailwind-css classes, see https://tailwindcss.com/ for more information
* For example: `center bg-gray-500 mx-2 my-1 rounded-full`
* inline: Apply CSS-classes <b>{value}</b> to the label
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS-classes to the label
* suggestions: return [{if: "value=bg-white rounded px-2", then: "Draw on a white background"}]
*/
labelCssClasses?: string | TagRenderingConfigJson
/**
* question: What CSS should be applied to the entire marker?
* You can set the css-properties here, e.g. `background: red; font-size: 12px; `
@ -102,7 +126,7 @@ export default interface PointRenderingConfigJson {
* inline: Apply CSS-style <b>{value}</b> to the _entire marker_
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS element to the entire marker
*
* group: expert
*/
css?: string | TagRenderingConfigJson
@ -117,34 +141,14 @@ export default interface PointRenderingConfigJson {
* ifunset: Do not apply extra CSS-classes to the label
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS-classes to the entire marker
* group: expert
*/
cssClasses?: string | TagRenderingConfigJson
/**
* question: What CSS should be applied to the label?
* You can set the css-properties here, e.g. `background: red; font-size: 12px; `
* inline: Apply CSS-style <b>{value}</b> to the label
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS-labels to the label
*
*/
labelCss?: TagRenderingConfigJson | string
/**
* question: Which CSS-classes should be applied to the label?
*
* The classes should be separated by a space (` `)
* You can use most Tailwind-css classes, see https://tailwindcss.com/ for more information
* For example: `center bg-gray-500 mx-2 my-1 rounded-full`
* inline: Apply CSS-classes <b>{value}</b> to the label
* types: Dynamic value ; string
* ifunset: Do not apply extra CSS-classes to the label
*/
labelCssClasses?: string | TagRenderingConfigJson
/**
* question: If the map is pitched, should the icon stay parallel to the screen or to the groundplane?
* suggestions: return [{if: "value=canvas", then: "The icon will stay upward and not be transformed as if it sticks to the screen"}, {if: "value=map", then: "The icon will be transformed as if it were painted onto the ground. (Automatically sets rotationAlignment)"}]
* group: expert
*/
pitchAlignment?: "canvas" | "map" | TagRenderingConfigJson
@ -152,6 +156,7 @@ export default interface PointRenderingConfigJson {
* question: Should the icon be rotated if the map is rotated?
* ifunset: Do not rotate or tilt icons. Always keep the icons straight
* suggestions: return [{if: "value=canvas", then: "Never rotate the icon"}, {if: "value=map", then: "If the map is rotated, rotate the icon as well. This gives the impression of an icon that floats perpendicular above the ground."}]
* group: expert
*/
rotationAlignment?: "map" | "canvas" | TagRenderingConfigJson
}

2
src/Models/ThemeConfig/Json/TagRenderingConfigJson.ts
View file

@ -56,6 +56,7 @@ export interface TagRenderingConfigJson {
*
* Note that this is a HTML-interpreted value, so you can add links as e.g. '&lt;a href='{website}'>{website}&lt;/a>' or include images such as `This is of type A &lt;br>&lt;img src='typeA-icon.svg' />`
* type: rendered
* ifunset: no text is rendered if no predefined options match
*/
render?:
| Translatable
@ -66,6 +67,7 @@ export interface TagRenderingConfigJson {
* An icon shown next to the rendering; typically shown pretty small
* This is only shown next to the "render" value
* Type: icon
* ifunset: do not show an icon next to the "render"-value
*/
icon?:
| string