From da29d82e016d042fad32bce20b96d78ec79eb6e0 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 12 Jun 2023 13:02:04 +0700 Subject: [PATCH] chore(developer): complete error checking in kmc-analyze --- common/web/types/src/main.ts | 2 +- .../src/osk-character-use/index.ts | 71 +++++++++++++------ developer/src/kmc-kmn/src/main.ts | 1 + developer/src/kmc/src/commands/analyze.ts | 9 ++- 4 files changed, 56 insertions(+), 27 deletions(-) diff --git a/common/web/types/src/main.ts b/common/web/types/src/main.ts index 67cf635816..214eabffb1 100644 --- a/common/web/types/src/main.ts +++ b/common/web/types/src/main.ts @@ -10,7 +10,7 @@ export { default as KvkFileReader } from './kvk/kvk-file-reader.js'; export { default as KvksFileReader } from './kvk/kvks-file-reader.js'; export { default as KvkFileWriter } from './kvk/kvk-file-writer.js'; export * as KvkFile from './kvk/kvk-file.js'; -export * as KvksFile from './kvk/kvk-file.js'; +export * as KvksFile from './kvk/kvks-file.js'; export * as LDMLKeyboard from './ldml-keyboard/ldml-keyboard-xml.js'; export { LDMLKeyboardTestDataXMLSourceFile } from './ldml-keyboard/ldml-keyboard-testdata-xml.js'; diff --git a/developer/src/kmc-analyze/src/osk-character-use/index.ts b/developer/src/kmc-analyze/src/osk-character-use/index.ts index d14197a3d5..f26a5c61d4 100644 --- a/developer/src/kmc-analyze/src/osk-character-use/index.ts +++ b/developer/src/kmc-analyze/src/osk-character-use/index.ts @@ -1,5 +1,5 @@ -import { CompilerCallbacks, KeymanDeveloperProject, KeymanFileTypes, KMX, KmxFileReader, KPJFileReader, KvksFileReader, TouchLayout, TouchLayoutFileReader } from "@keymanapp/common-types"; -import { KmnCompiler } from '@keymanapp/kmc-kmn'; +import { CompilerCallbacks, KeymanDeveloperProject, KeymanFileTypes, KMX, KmxFileReader, KPJFileReader, KvksFile, KvksFileReader, TouchLayout, TouchLayoutFileReader } from "@keymanapp/common-types"; +import { KmnCompiler, CompilerMessages } from '@keymanapp/kmc-kmn'; import { AnalyzerMessages } from "../messages.js"; export class AnalyzeOskCharacterUse { @@ -16,25 +16,39 @@ export class AnalyzeOskCharacterUse { // Analyze a set of files // - public async analyze(files: string[], analyzeProjects: boolean = true) { + public async analyze(files: string[], analyzeProjects: boolean = true): Promise { for(let file of files) { switch(KeymanFileTypes.sourceTypeFromFilename(file)) { - case KeymanFileTypes.Source.VisualKeyboard: - this.addStrings(this.scanVisualKeyboard(file)); + case KeymanFileTypes.Source.VisualKeyboard: { + let strings = this.scanVisualKeyboard(file); + if(!strings) { + return false; + } + this.addStrings(strings); break; - case KeymanFileTypes.Source.TouchLayout: - this.addStrings(this.scanTouchLayout(file)); + } + case KeymanFileTypes.Source.TouchLayout: { + let strings = this.scanTouchLayout(file); + if(!strings) { + return false; + } + this.addStrings(strings); break; + } case KeymanFileTypes.Source.Project: if(analyzeProjects) { - await this.analyzeProject(file); + if(!await this.analyzeProject(file)) { + return false; + } } break; case KeymanFileTypes.Source.KeymanKeyboard: // The cleanest way to do this is to compile the .kmn to find the .kvks // and .keyman-touch-layout from the &VISUALKEYBOARD and &LAYOUTFILE // system stores - await this.analyzeKmnKeyboard(file); + if(!await this.analyzeKmnKeyboard(file)) { + return false; + } break; case KeymanFileTypes.Source.LdmlKeyboard: // TODO: await this.analyzeLdmlKeyboard(file); @@ -45,17 +59,18 @@ export class AnalyzeOskCharacterUse { // easily also } } + return true; } - private async analyzeProject(filename: string): Promise { + private async analyzeProject(filename: string): Promise { const reader = new KPJFileReader(this.callbacks); const source = reader.read(this.callbacks.loadFile(filename)); const project = reader.transform(filename, source); let files = project.files.map(file => this.callbacks.resolveFilename(filename, file.filePath)); - await this.analyze(files, false); // false because we don't want get into a recursive loop for projects + return await this.analyze(files, false); // false because we don't want get into a recursive loop for projects } - public async analyzeProjectFolder(folder: string) { + public async analyzeProjectFolder(folder: string): Promise { // TODO: consider reworking slightly alongside kmc build BuildProject, with // common file vs folder logic to be refactored to be a shared helper // function, probably with the KpjFileReader? @@ -64,7 +79,7 @@ export class AnalyzeOskCharacterUse { if(this.callbacks.fs.existsSync(kpjFile)) { this.callbacks.reportMessage(AnalyzerMessages.Info_ScanningFile({type:'project', name:kpjFile})); - await this.analyzeProject(kpjFile); + return await this.analyzeProject(kpjFile); } else { this.callbacks.reportMessage(AnalyzerMessages.Info_ScanningFile({type:'project folder', name:folder})); const project = new KeymanDeveloperProject(kpjFile, '2.0', this.callbacks); @@ -75,18 +90,17 @@ export class AnalyzeOskCharacterUse { // accidentally end up recursing files = files.filter(file => !KeymanFileTypes.filenameIs(file, KeymanFileTypes.Source.Project)); - await this.analyze(files, false); // false because we don't want get into a recursive loop for projects + return await this.analyze(files, false); // false because we don't want get into a recursive loop for projects } } - private async analyzeKmnKeyboard(filename: string): Promise { + private async analyzeKmnKeyboard(filename: string): Promise { this.callbacks.reportMessage(AnalyzerMessages.Info_ScanningFile({type:'keyboard source', name:filename})); const kmnCompiler = new KmnCompiler(); if(!await kmnCompiler.init(this.callbacks)) { - // TODO: error handling - console.error('kmx compiler failed to init'); - process.exit(1); + // kmnCompiler will report errors + return false; } // Note, output filename here is just to provide path data, @@ -98,8 +112,8 @@ export class AnalyzeOskCharacterUse { }); if(!result) { - //TODO: error handling - process.exit(1); + // kmnCompiler will report any errors + return false; } if(result.data.kvksFilename) { @@ -113,6 +127,8 @@ export class AnalyzeOskCharacterUse { if(touchLayoutStore) { this.addStrings(this.scanTouchLayout(this.callbacks.resolveFilename(filename, touchLayoutStore.dpString))); } + + return true; } private addStrings(strings: string[]) { @@ -127,10 +143,19 @@ export class AnalyzeOskCharacterUse { this.callbacks.reportMessage(AnalyzerMessages.Info_ScanningFile({type:'visual keyboard', name:filename})); let strings: string[] = []; const reader = new KvksFileReader(); - const source = reader.read(this.callbacks.loadFile(filename)); + let source: KvksFile.default; + try { + source = reader.read(this.callbacks.loadFile(filename)); + } catch(e) { + this.callbacks.reportMessage(CompilerMessages.Error_InvalidKvksFile({filename, e})); + return null; + } let invalidKeys: string[] = []; const vk = reader.transform(source, invalidKeys); - // TODO check vk, invalidKeys + if(!vk) { + this.callbacks.reportMessage(CompilerMessages.Error_InvalidKvksFile({filename, e:null})); + return null; + } for(let key of vk.keys) { if(key.text) { strings.push(key.text); @@ -144,7 +169,7 @@ export class AnalyzeOskCharacterUse { let strings: string[] = []; const reader = new TouchLayoutFileReader(); const source = reader.read(this.callbacks.loadFile(filename)); - // TODO: handle errors + const scanKey = (key: TouchLayout.TouchLayoutKey | TouchLayout.TouchLayoutSubKey) => { if(!key.text) { return; diff --git a/developer/src/kmc-kmn/src/main.ts b/developer/src/kmc-kmn/src/main.ts index 70d11989c6..b26dd23c80 100644 --- a/developer/src/kmc-kmn/src/main.ts +++ b/developer/src/kmc-kmn/src/main.ts @@ -1,2 +1,3 @@ export { KmnCompiler } from './compiler/compiler.js'; +export { CompilerMessages } from './compiler/messages.js'; \ No newline at end of file diff --git a/developer/src/kmc/src/commands/analyze.ts b/developer/src/kmc/src/commands/analyze.ts index 533403d9db..7785bb7804 100644 --- a/developer/src/kmc/src/commands/analyze.ts +++ b/developer/src/kmc/src/commands/analyze.ts @@ -27,7 +27,6 @@ export function declareAnalyze(program: Command) { if(!analyze(filenames, options)) { // Once a file fails to build, we bail on subsequent builds - // TODO: is this the most appropriate semantics? process.exit(1); } }); @@ -61,10 +60,14 @@ async function analyzeOskCharUse(callbacks: CompilerCallbacks, filenames: string // If infile is a directory, then we treat that as a project and build it if(fs.statSync(filename).isDirectory()) { - await analyzer.analyzeProjectFolder(filename); + if(!await analyzer.analyzeProjectFolder(filename)) { + return false; + } } else { - await analyzer.analyze([filename]); + if(!await analyzer.analyze([filename])) { + return false; + } } }