From 48c0f09872d4490bb9dd99f49482866969918a89 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 2 Aug 2023 11:50:35 +0700 Subject: [PATCH 1/3] chore(developer): use getFileType() instead of fileType Ensures that we calculate the file type instead of using stored data, which is a lot cleaner. Means we ignore the filetype field in the .kpj xml, which is fine. We should do the same with filename vs filepath. --- .../types/src/kpj/keyman-developer-project.ts | 21 +++++-------------- common/web/types/src/kpj/kpj-file-reader.ts | 3 +-- .../types/test/kpj/test-kpj-file-reader.ts | 8 +++---- .../src/commands/buildClasses/BuildProject.ts | 2 +- 4 files changed, 11 insertions(+), 23 deletions(-) diff --git a/common/web/types/src/kpj/keyman-developer-project.ts b/common/web/types/src/kpj/keyman-developer-project.ts index 841111e00a..b7e0195b5d 100644 --- a/common/web/types/src/kpj/keyman-developer-project.ts +++ b/common/web/types/src/kpj/keyman-developer-project.ts @@ -43,11 +43,11 @@ export class KeymanDeveloperProject { } public isKeyboardProject() { - return !!this.files.find(file => file.fileType == KeymanFileTypes.Source.KeymanKeyboard || file.fileType == KeymanFileTypes.Source.LdmlKeyboard); + return !!this.files.find(file => file.getFileType() == KeymanFileTypes.Source.KeymanKeyboard || file.getFileType() == KeymanFileTypes.Source.LdmlKeyboard); } public isLexicalModelProject() { - return !!this.files.find(file => file.fileType == KeymanFileTypes.Source.Model); + return !!this.files.find(file => file.getFileType() == KeymanFileTypes.Source.Model); } public addMetadataFile() { @@ -134,21 +134,16 @@ export class KeymanDeveloperProjectFile10 { readonly filename: string; readonly filePath: string; readonly fileVersion: string; // 1.0 only - /** - * file extension of filename; warning: .model.ts is technically not the fileType because of 2 periods - * @deprecated use `getFileType()` - */ - readonly fileType: KeymanFileTypes.Any; // 1.0 only details: KeymanDeveloperProjectFileDetail_Kmn & KeymanDeveloperProjectFileDetail_Kps; // 1.0 only childFiles: KeymanDeveloperProjectFile[]; // 1.0 only - constructor(id: string, filename: string, filePath: string, fileVersion:string, fileType: KeymanFileTypes.Any) { + constructor(id: string, filename: string, filePath: string, fileVersion:string) { this.details = {}; this.childFiles = []; this.id = id; this.filename = filename; this.filePath = filePath; this.fileVersion = fileVersion; - this.fileType = fileType; + // we now ignore the fileType on load } getFileType() { return KeymanFileTypes.sourceTypeFromFilename(this.filename); @@ -160,18 +155,12 @@ export type KeymanDeveloperProjectFileType20 = KeymanFileTypes.Source; export class KeymanDeveloperProjectFile20 { readonly filename: string; readonly filePath: string; - /** - * file extension of filename, but .model.ts is technically not the ext because of 2 periods - * @deprecated TODO: remove this from 2.0 or make it private - */ - readonly fileType: KeymanFileTypes.Source; constructor(filePath: string, private callbacks: CompilerCallbacks) { this.filename = this.callbacks.path.basename(filePath); this.filePath = filePath; - this.fileType = KeymanFileTypes.sourceTypeFromFilename(this.filename); } getFileType() { - return this.fileType; + return KeymanFileTypes.sourceTypeFromFilename(this.filename); } }; diff --git a/common/web/types/src/kpj/kpj-file-reader.ts b/common/web/types/src/kpj/kpj-file-reader.ts index 9e3b934c35..745eab8fa5 100644 --- a/common/web/types/src/kpj/kpj-file-reader.ts +++ b/common/web/types/src/kpj/kpj-file-reader.ts @@ -90,8 +90,7 @@ export class KPJFileReader { sourceFile.ID || '', sourceFile.Filename || '', (sourceFile.Filepath || '').replace(/\\/g, '/'), - sourceFile.FileVersion || '', - sourceFile.FileType || '' + sourceFile.FileVersion || '' ); if (sourceFile.Details) { file.details.copyright = sourceFile.Details.Copyright; diff --git a/common/web/types/test/kpj/test-kpj-file-reader.ts b/common/web/types/test/kpj/test-kpj-file-reader.ts index 2c3a5c383b..34cdb25949 100644 --- a/common/web/types/test/kpj/test-kpj-file-reader.ts +++ b/common/web/types/test/kpj/test-kpj-file-reader.ts @@ -82,7 +82,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.kmn'); assert.equal(f.filePath, 'source/khmer_angkor.kmn'); assert.equal(f.fileVersion, '1.3'); - assert.equal(f.fileType, '.kmn'); + assert.equal(f.getFileType(), '.kmn'); assert.equal(f.details.name, 'Khmer Angkor'); assert.equal(f.details.copyright, '© 2015-2022 SIL International'); assert.equal(f.details.message, 'More than just a Khmer Unicode keyboard.'); @@ -94,7 +94,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.kps'); assert.equal(f.filePath, 'source/khmer_angkor.kps'); assert.equal(f.fileVersion, ''); - assert.equal(f.fileType, '.kps'); + assert.equal(f.getFileType(), '.kps'); assert.equal(f.details.name, 'Khmer Angkor'); assert.isUndefined(f.details.message); assert.equal(f.details.copyright, '© 2015-2022 SIL International'); @@ -107,7 +107,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.ico'); assert.equal(f.filePath, 'source/khmer_angkor.ico'); assert.equal(f.fileVersion, ''); - assert.equal(f.fileType, '.ico'); + assert.equal(f.getFileType(), '.ico'); assert.isEmpty(f.details); assert.lengthOf(f.childFiles, 0); @@ -117,7 +117,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.js'); assert.equal(f.filePath, 'source/../build/khmer_angkor.js'); assert.equal(f.fileVersion, ''); - assert.equal(f.fileType, '.js'); + assert.equal(f.getFileType(), '.js'); assert.isEmpty(f.details); assert.lengthOf(f.childFiles, 0); }); diff --git a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts index 44cb990704..22d96f7e68 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts @@ -61,7 +61,7 @@ class ProjectBuilder { async buildProjectTargets(activity: BuildActivity): Promise { let result = true; for(let file of this.project.files) { - if(file.fileType.toLowerCase() == activity.sourceExtension) { + if(file.getFileType().toLowerCase() == activity.sourceExtension) { result = await this.buildTarget(file, activity) && result; } } From d8e03e069519f11afdd5cdee3ed02f822fb4065f Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 2 Aug 2023 12:05:48 +0700 Subject: [PATCH 2/3] chore(developer): refactor KeymanDeveloperProjectFile Turns KeymanDeveloperProjectFile into an interface, and removes redundant data in filename and fileType fields, calculating these from filePath instead. Maintains separate KeymanDeveloperProjectFile10 and KeymanDeveloperProjectFile20 classes implementing the base interface for now, although that may be a target for future consolidation (lowpri). --- .../types/src/kpj/keyman-developer-project.ts | 46 +++++++++---------- common/web/types/src/kpj/kpj-file-reader.ts | 4 +- .../types/test/kpj/test-kpj-file-reader.ts | 8 ++-- .../buildClasses/BuildKeyboardInfo.ts | 2 +- .../commands/buildClasses/BuildModelInfo.ts | 6 +-- .../src/commands/buildClasses/BuildProject.ts | 2 +- 6 files changed, 33 insertions(+), 35 deletions(-) diff --git a/common/web/types/src/kpj/keyman-developer-project.ts b/common/web/types/src/kpj/keyman-developer-project.ts index b7e0195b5d..9ba1b299c0 100644 --- a/common/web/types/src/kpj/keyman-developer-project.ts +++ b/common/web/types/src/kpj/keyman-developer-project.ts @@ -43,11 +43,11 @@ export class KeymanDeveloperProject { } public isKeyboardProject() { - return !!this.files.find(file => file.getFileType() == KeymanFileTypes.Source.KeymanKeyboard || file.getFileType() == KeymanFileTypes.Source.LdmlKeyboard); + return !!this.files.find(file => file.fileType == KeymanFileTypes.Source.KeymanKeyboard || file.fileType == KeymanFileTypes.Source.LdmlKeyboard); } public isLexicalModelProject() { - return !!this.files.find(file => file.getFileType() == KeymanFileTypes.Source.Model); + return !!this.files.find(file => file.fileType == KeymanFileTypes.Source.Model); } public addMetadataFile() { @@ -127,41 +127,39 @@ export class KeymanDeveloperProjectOptions { } }; -export type KeymanDeveloperProjectFile = KeymanDeveloperProjectFile10 | KeymanDeveloperProjectFile20; - -export class KeymanDeveloperProjectFile10 { - readonly id: string; // 1.0 only - readonly filename: string; +export interface KeymanDeveloperProjectFile { + get filename(): string; + get fileType(): string; readonly filePath: string; - readonly fileVersion: string; // 1.0 only +}; + +export class KeymanDeveloperProjectFile10 implements KeymanDeveloperProjectFile { + get filename(): string { + return this.callbacks.path.basename(this.filePath); + } + get fileType(): string { + return KeymanFileTypes.sourceTypeFromFilename(this.filename); + } details: KeymanDeveloperProjectFileDetail_Kmn & KeymanDeveloperProjectFileDetail_Kps; // 1.0 only childFiles: KeymanDeveloperProjectFile[]; // 1.0 only - constructor(id: string, filename: string, filePath: string, fileVersion:string) { + + constructor(public readonly id: string, public readonly filePath: string, public readonly fileVersion:string, private readonly callbacks: CompilerCallbacks) { this.details = {}; this.childFiles = []; - this.id = id; - this.filename = filename; - this.filePath = filePath; - this.fileVersion = fileVersion; - // we now ignore the fileType on load - } - getFileType() { - return KeymanFileTypes.sourceTypeFromFilename(this.filename); } }; export type KeymanDeveloperProjectFileType20 = KeymanFileTypes.Source; -export class KeymanDeveloperProjectFile20 { - readonly filename: string; - readonly filePath: string; - constructor(filePath: string, private callbacks: CompilerCallbacks) { - this.filename = this.callbacks.path.basename(filePath); - this.filePath = filePath; +export class KeymanDeveloperProjectFile20 implements KeymanDeveloperProjectFile { + get filename(): string { + return this.callbacks.path.basename(this.filePath); } - getFileType() { + get fileType() { return KeymanFileTypes.sourceTypeFromFilename(this.filename); } + constructor(public readonly filePath: string, private readonly callbacks: CompilerCallbacks) { + } }; export class KeymanDeveloperProjectFileDetail_Kps { diff --git a/common/web/types/src/kpj/kpj-file-reader.ts b/common/web/types/src/kpj/kpj-file-reader.ts index 745eab8fa5..2c8589b5c1 100644 --- a/common/web/types/src/kpj/kpj-file-reader.ts +++ b/common/web/types/src/kpj/kpj-file-reader.ts @@ -88,9 +88,9 @@ export class KPJFileReader { for (let sourceFile of project.Files?.File) { let file: KeymanDeveloperProjectFile10 = new KeymanDeveloperProjectFile10( sourceFile.ID || '', - sourceFile.Filename || '', (sourceFile.Filepath || '').replace(/\\/g, '/'), - sourceFile.FileVersion || '' + sourceFile.FileVersion || '', + this.callbacks ); if (sourceFile.Details) { file.details.copyright = sourceFile.Details.Copyright; diff --git a/common/web/types/test/kpj/test-kpj-file-reader.ts b/common/web/types/test/kpj/test-kpj-file-reader.ts index 34cdb25949..2c3a5c383b 100644 --- a/common/web/types/test/kpj/test-kpj-file-reader.ts +++ b/common/web/types/test/kpj/test-kpj-file-reader.ts @@ -82,7 +82,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.kmn'); assert.equal(f.filePath, 'source/khmer_angkor.kmn'); assert.equal(f.fileVersion, '1.3'); - assert.equal(f.getFileType(), '.kmn'); + assert.equal(f.fileType, '.kmn'); assert.equal(f.details.name, 'Khmer Angkor'); assert.equal(f.details.copyright, '© 2015-2022 SIL International'); assert.equal(f.details.message, 'More than just a Khmer Unicode keyboard.'); @@ -94,7 +94,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.kps'); assert.equal(f.filePath, 'source/khmer_angkor.kps'); assert.equal(f.fileVersion, ''); - assert.equal(f.getFileType(), '.kps'); + assert.equal(f.fileType, '.kps'); assert.equal(f.details.name, 'Khmer Angkor'); assert.isUndefined(f.details.message); assert.equal(f.details.copyright, '© 2015-2022 SIL International'); @@ -107,7 +107,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.ico'); assert.equal(f.filePath, 'source/khmer_angkor.ico'); assert.equal(f.fileVersion, ''); - assert.equal(f.getFileType(), '.ico'); + assert.equal(f.fileType, '.ico'); assert.isEmpty(f.details); assert.lengthOf(f.childFiles, 0); @@ -117,7 +117,7 @@ describe('kpj-file-reader', function () { assert.equal(f.filename, 'khmer_angkor.js'); assert.equal(f.filePath, 'source/../build/khmer_angkor.js'); assert.equal(f.fileVersion, ''); - assert.equal(f.getFileType(), '.js'); + assert.equal(f.fileType, '.js'); assert.isEmpty(f.details); assert.lengthOf(f.childFiles, 0); }); diff --git a/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts b/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts index decc5197e5..14670c99b4 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildKeyboardInfo.ts @@ -81,7 +81,7 @@ export class BuildKeyboardInfo extends BuildActivity { } function findProjectFile(callbacks: CompilerCallbacks, project: KeymanDeveloperProject, ext: KeymanFileTypes.Source) { - const file = project.files.find(file => file.getFileType() == ext); + const file = project.files.find(file => file.fileType == ext); if(!file) { callbacks.reportMessage(InfrastructureMessages.Error_FileTypeNotFound({ext})); } diff --git a/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts b/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts index c20f038369..bfb167bdcc 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildModelInfo.ts @@ -35,19 +35,19 @@ export class BuildModelInfo extends BuildActivity { return false; } - const metadata = project.files.find(file => file.getFileType() == KeymanFileTypes.Source.ModelInfo); + const metadata = project.files.find(file => file.fileType == KeymanFileTypes.Source.ModelInfo); if(!metadata) { callbacks.reportMessage(InfrastructureMessages.Error_FileTypeNotFound({ext: KeymanFileTypes.Source.ModelInfo})); return false; } - const model = project.files.find(file => file.getFileType() == KeymanFileTypes.Source.Model); + const model = project.files.find(file => file.fileType == KeymanFileTypes.Source.Model); if(!model) { callbacks.reportMessage(InfrastructureMessages.Error_FileTypeNotFound({ext: KeymanFileTypes.Source.Model})); return false; } - const kps = project.files.find(file => file.getFileType() == KeymanFileTypes.Source.Package); + const kps = project.files.find(file => file.fileType == KeymanFileTypes.Source.Package); if(!kps) { callbacks.reportMessage(InfrastructureMessages.Error_FileTypeNotFound({ext: KeymanFileTypes.Source.Package})); return false; diff --git a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts index 22d96f7e68..44cb990704 100644 --- a/developer/src/kmc/src/commands/buildClasses/BuildProject.ts +++ b/developer/src/kmc/src/commands/buildClasses/BuildProject.ts @@ -61,7 +61,7 @@ class ProjectBuilder { async buildProjectTargets(activity: BuildActivity): Promise { let result = true; for(let file of this.project.files) { - if(file.getFileType().toLowerCase() == activity.sourceExtension) { + if(file.fileType.toLowerCase() == activity.sourceExtension) { result = await this.buildTarget(file, activity) && result; } } From c20a670f91ae0e4eef2df746cfb52e2a03bf0260 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 2 Aug 2023 12:32:07 +0700 Subject: [PATCH 3/3] chore(developer): handle unknown file extensions in file types Projects can contain any type of file, but the fromFilename file type utility function would only handle known source and binary file types. --- .../types/src/kpj/keyman-developer-project.ts | 4 ++-- common/web/types/src/util/file-types.ts | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/common/web/types/src/kpj/keyman-developer-project.ts b/common/web/types/src/kpj/keyman-developer-project.ts index 9ba1b299c0..ce9f78cf59 100644 --- a/common/web/types/src/kpj/keyman-developer-project.ts +++ b/common/web/types/src/kpj/keyman-developer-project.ts @@ -138,7 +138,7 @@ export class KeymanDeveloperProjectFile10 implements KeymanDeveloperProjectFile return this.callbacks.path.basename(this.filePath); } get fileType(): string { - return KeymanFileTypes.sourceTypeFromFilename(this.filename); + return KeymanFileTypes.fromFilename(this.filename); } details: KeymanDeveloperProjectFileDetail_Kmn & KeymanDeveloperProjectFileDetail_Kps; // 1.0 only childFiles: KeymanDeveloperProjectFile[]; // 1.0 only @@ -156,7 +156,7 @@ export class KeymanDeveloperProjectFile20 implements KeymanDeveloperProjectFile return this.callbacks.path.basename(this.filePath); } get fileType() { - return KeymanFileTypes.sourceTypeFromFilename(this.filename); + return KeymanFileTypes.fromFilename(this.filename); } constructor(public readonly filePath: string, private readonly callbacks: CompilerCallbacks) { } diff --git a/common/web/types/src/util/file-types.ts b/common/web/types/src/util/file-types.ts index 230792d9b7..ebad759638 100644 --- a/common/web/types/src/util/file-types.ts +++ b/common/web/types/src/util/file-types.ts @@ -68,6 +68,23 @@ export type All = Source | Binary; */ export type Any = string; +/** + * Gets the file type based on extension, dealing with multi-part file + * extensions. Does not sniff contents of file or assume file existence. Does + * transform upper-cased file extensions to lower-case. + * @param filename + * @returns file extension, or `""` if no extension. Note that this return value + * differs from the other, more-specific fromFilename functions below, + * which return `null` if a supported extension is not found. + */ +export function fromFilename(filename: string): Binary | Source | Any { + const result = + sourceOrBinaryTypeFromFilename(filename) ?? + filename.match(/\.[^\.]+$/)?.[0] ?? + ""; + return result; +} + /** * Gets the file type based on extension, dealing with multi-part file * extensions. Does not sniff contents of file or assume file existence. @@ -75,7 +92,7 @@ export type Any = string; * @param filename * @returns file type, or `null` if not found */ -export function fromFilename(filename: string): Binary | Source { +export function sourceOrBinaryTypeFromFilename(filename: string): Binary | Source { filename = filename.toLowerCase(); const result = ALL_SOURCE.find(type => filename.endsWith(type)) ??