From f27b60f80d9dbd7dba63aac1fe90a29e3260c8a2 Mon Sep 17 00:00:00 2001 From: pietervdvn Date: Wed, 6 Jul 2022 11:14:19 +0200 Subject: [PATCH] Better errors when an image is missing --- Models/ThemeConfig/Conversion/Conversion.ts | 8 +++ Models/ThemeConfig/Conversion/Validation.ts | 76 +++++++++++++++------ scripts/generateLayerOverview.ts | 4 +- 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/Models/ThemeConfig/Conversion/Conversion.ts b/Models/ThemeConfig/Conversion/Conversion.ts index 5fe52ebb0..2b7880c1f 100644 --- a/Models/ThemeConfig/Conversion/Conversion.ts +++ b/Models/ThemeConfig/Conversion/Conversion.ts @@ -39,6 +39,14 @@ export abstract class Conversion { return DesugaringStep.strict(fixed) } + public convertJoin(json: TIn, context: string, errors: string[], warnings?: string[], information?: string[]): TOut { + const fixed = this.convert(json, context) + errors?.push(...(fixed.errors ?? [])) + warnings?.push(...(fixed.warnings ?? [])) + information?.push(...(fixed.information ?? [])) + return fixed.result + } + public andThenF(f: (tout:TOut) => X ): Conversion{ return new Pipe( this, diff --git a/Models/ThemeConfig/Conversion/Validation.ts b/Models/ThemeConfig/Conversion/Validation.ts index 79e4785f3..47250c36b 100644 --- a/Models/ThemeConfig/Conversion/Validation.ts +++ b/Models/ThemeConfig/Conversion/Validation.ts @@ -45,6 +45,53 @@ class ValidateLanguageCompleteness extends DesugaringStep { } } +export class DoesImageExist extends DesugaringStep{ + + private readonly _knownImagePaths: Set; + public static doesPathExist : (path: string) => boolean = undefined; + + constructor(knownImagePaths: Set) { + super("Checks if an image exists", [], "DoesImageExist"); + this._knownImagePaths = knownImagePaths; + } + + convert(image: string, context: string): { result: string; errors?: string[]; warnings?: string[]; information?: string[] } { + const errors = [] + const warnings = [] + const information = [] + if (image.indexOf("{") >= 0) { + information.push("Ignoring image with { in the path: " + image) + return {result: image} + } + + if (image === "assets/SocialImage.png") { + return {result: image} + } + if (image.match(/[a-z]*/)) { + + if (Svg.All[image + ".svg"] !== undefined) { + // This is a builtin img, e.g. 'checkmark' or 'crosshair' + return {result: image}; + } + } + + if (this._knownImagePaths !== undefined && !this._knownImagePaths.has(image)) { + if(DoesImageExist.doesPathExist === undefined){ + errors.push(`Image with path ${image} not found or not attributed; it is used in ${context}`) + }else if(!DoesImageExist.doesPathExist(image)){ + errors.push(`Image with path ${image} does not exist; it is used in ${context}.\n Check for typo's and missing directories in the path.`) + }else{ + errors.push(`Image with path ${image} is not attributed (but it exists); execute 'npm run query:licenses' to add the license information and/or run 'npm run generate:licenses' to compile all the license info`) + } + } + return { + result: image, + errors, warnings, information + } + } + +} + class ValidateTheme extends DesugaringStep { /** * The paths where this layer is originally saved. Triggers some extra checks @@ -54,13 +101,14 @@ class ValidateTheme extends DesugaringStep { private readonly knownImagePaths: Set; private readonly _isBuiltin: boolean; private _sharedTagRenderings: Map; - + private readonly _validateImage : DesugaringStep; constructor(knownImagePaths: Set, path: string, isBuiltin: boolean, sharedTagRenderings: Map) { super("Doesn't change anything, but emits warnings and errors", [], "ValidateTheme"); this.knownImagePaths = knownImagePaths; this._path = path; this._isBuiltin = isBuiltin; this._sharedTagRenderings = sharedTagRenderings; + this._validateImage = new DoesImageExist(this.knownImagePaths); } convert(json: LayoutConfigJson, context: string): { result: LayoutConfigJson; errors: string[], warnings: string[], information: string[] } { @@ -89,26 +137,7 @@ class ValidateTheme extends DesugaringStep { errors.push("Found a remote image: " + remoteImage + " in theme " + json.id + ", please download it.") } for (const image of images) { - if (image.indexOf("{") >= 0) { - information.push("Ignoring image with { in the path: " + image) - continue - } - - if (image === "assets/SocialImage.png") { - continue - } - if (image.match(/[a-z]*/)) { - - if (Svg.All[image + ".svg"] !== undefined) { - // This is a builtin img, e.g. 'checkmark' or 'crosshair' - continue;// => - } - } - - if (this.knownImagePaths !== undefined && !this.knownImagePaths.has(image)) { - const ctx = context === undefined ? "" : ` in a layer defined in the theme ${context}` - errors.push(`Image with path ${image} not found or not attributed; it is used in ${json.id}${ctx}`) - } + this._validateImage.convertJoin(image, context === undefined ? "" : ` in a layer defined in the theme ${context}`, errors, warnings, information) } if (json.icon.endsWith(".svg")) { @@ -433,8 +462,11 @@ export class DetectMappingsWithImages extends DesugaringStep i); + const ctx = context === undefined ? "" : ` in a layer defined in the theme ${context}` - errors.push(`Image with path ${image} not found or not attributed; it is used in ${json.id}${ctx}`) + errors.push(`Image with path ${image} not found or not attributed; it is used in ${json.id}${ctx}.\n Did you mean one of ${closeNames.slice(0, 3).join(", ")}`) } } diff --git a/scripts/generateLayerOverview.ts b/scripts/generateLayerOverview.ts index 81a67a69a..9a8d762e8 100644 --- a/scripts/generateLayerOverview.ts +++ b/scripts/generateLayerOverview.ts @@ -5,6 +5,7 @@ import {LayoutConfigJson} from "../Models/ThemeConfig/Json/LayoutConfigJson"; import {LayerConfigJson} from "../Models/ThemeConfig/Json/LayerConfigJson"; import Constants from "../Models/Constants"; import { + DoesImageExist, PrevalidateTheme, ValidateLayer, ValidateTagRenderings, @@ -152,6 +153,8 @@ class LayerOverviewUtils { main(_: string[]) { + DoesImageExist.doesPathExist = (path) => existsSync(path) + const licensePaths = new Set() for (const i in licenses) { licensePaths.add(licenses[i].path) @@ -261,7 +264,6 @@ class LayerOverviewUtils { tagRenderings: this.getSharedTagRenderings(knownImagePaths), publicLayers } - const nonDefaultLanguages : {theme: string, language: string}[] = [] for (const themeInfo of themeFiles) { let themeFile = themeInfo.parsed const themePath = themeInfo.path