From 1f19e6e4596b8dd4fcec498bf9c6292467e2ec9f Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Wed, 6 Dec 2023 21:06:44 -0800 Subject: [PATCH 1/8] Draft --- src/modules/argumentsParser.ts | 11 +++++++- src/types/options.ts | 8 ++++++ src/types/outputFactory.ts | 28 +++++++++++++++------ test/modules/argumentsParser.test.ts | 9 +++++++ test/modules/candidatePostProcessor.test.ts | 1 + test/outputFactory.test.ts | 26 +++++++++++++++---- 6 files changed, 70 insertions(+), 13 deletions(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index d03eeea2c..ddd42d71a 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -279,9 +279,18 @@ export default class ArgumentsParser { }) .option('dir-letter', { group: groupRomOutput, - description: 'Append the first letter of the ROM name as an output subdirectory', + description: 'Group games in an output subdirectory by the first letters in their name', type: 'boolean', }) + .option('dir-letter-count', { + group: groupRomOutput, + description: 'How many game name letters to use for the subdirectory name', + type: 'number', + coerce: (val: number) => Math.max(ArgumentsParser.getLastValue(val), 1), + requiresArg: true, + // Note: can't `implies: 'dir-letter'` with a default value set + default: 1, + }) .option('dir-letter-limit', { group: groupRomOutput, description: 'Limit the number ROMs in letter subdirectories, splitting into multiple if necessary', diff --git a/src/types/options.ts b/src/types/options.ts index b98d964a3..9da499954 100644 --- a/src/types/options.ts +++ b/src/types/options.ts @@ -66,6 +66,7 @@ export interface OptionsProps { readonly dirDatName?: boolean, readonly dirDatDescription?: boolean, readonly dirLetter?: boolean, + readonly dirLetterCount?: number, readonly dirLetterLimit?: number, readonly dirGameSubdir?: string, readonly overwrite?: boolean, @@ -181,6 +182,8 @@ export default class Options implements OptionsProps { readonly dirLetter: boolean; + readonly dirLetterCount: number; + readonly dirLetterLimit: number; readonly dirGameSubdir?: string; @@ -333,6 +336,7 @@ export default class Options implements OptionsProps { this.dirDatName = options?.dirDatName ?? false; this.dirDatDescription = options?.dirDatDescription ?? false; this.dirLetter = options?.dirLetter ?? false; + this.dirLetterCount = options?.dirLetterCount ?? 0; this.dirLetterLimit = options?.dirLetterLimit ?? 0; this.dirGameSubdir = options?.dirGameSubdir; this.overwrite = options?.overwrite ?? false; @@ -749,6 +753,10 @@ export default class Options implements OptionsProps { return this.dirLetter; } + getDirLetterCount(): number { + return this.dirLetterCount; + } + getDirLetterLimit(): number { return this.dirLetterLimit; } diff --git a/src/types/outputFactory.ts b/src/types/outputFactory.ts index 07b056432..09dd8798d 100644 --- a/src/types/outputFactory.ts +++ b/src/types/outputFactory.ts @@ -328,17 +328,31 @@ export default class OutputFactory { // Find the letter for every ROM filename let lettersToFilenames = (romBasenames ?? [romBasename]).reduce((map, filename) => { - let letter = filename.charAt(0).toUpperCase(); - if (letter.match(/[^A-Z]/)) { - letter = '#'; - } - - const existing = map.get(letter) ?? new Set(); + const filenameParsed = path.parse(filename); + let letters = (filenameParsed.dir || filenameParsed.name) + .substring(0, options.getDirLetterCount()) + .padEnd(options.getDirLetterCount(), 'A') + .toUpperCase() + .replace(/[^A-Z0-9]/g, '#'); + // TODO(cemmer): only do this when not grouping letters together + letters = letters.replace(/[^A-Z]/g, '#'); + + const existing = map.get(letters) ?? new Set(); existing.add(filename); - map.set(letter, existing); + map.set(letters, existing); return map; }, new Map>()); + [...lettersToFilenames.entries()] + .sort((a, b) => a[0].localeCompare(b[0])) + .reduce((arr, [letter, filenames]) => { + // TODO: group letters together + const tuples = [...filenames.values()] + .sort() + .map((filename) => [letter, filename] satisfies [string, string]); + return [...arr, ...tuples]; + }, [] as [string, string][]); + // Split the letter directories, if needed if (options.getDirLetterLimit()) { lettersToFilenames = [...lettersToFilenames.entries()] diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts index d70b9b42c..38434a9bf 100644 --- a/test/modules/argumentsParser.test.ts +++ b/test/modules/argumentsParser.test.ts @@ -108,6 +108,7 @@ describe('options', () => { expect(options.getDirDatDescription()).toEqual(false); expect(options.getDirLetter()).toEqual(false); expect(options.getDirLetterLimit()).toEqual(0); + expect(options.getDirLetterCount()).toEqual(1); expect(options.getDirGameSubdir()).toEqual(GameSubdirMode.MULTIPLE); expect(options.getOverwrite()).toEqual(false); expect(options.getOverwriteInvalid()).toEqual(false); @@ -342,6 +343,14 @@ describe('options', () => { expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', 'true', '--dir-letter', 'false']).getDirLetter()).toEqual(false); }); + it('should parse "dir-letter-count"', () => { + expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '-1']).getDirLetterCount()).toEqual(1); + expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '0']).getDirLetterCount()).toEqual(1); + expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '1']).getDirLetterCount()).toEqual(1); + expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '5']).getDirLetterCount()).toEqual(5); + expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '5', '--dir-letter-count', '10']).getDirLetterCount()).toEqual(10); + }); + it('should parse "dir-letter-limit"', () => { expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-limit'])).toThrow(/not enough arguments/i); expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-limit', '1'])).toThrow(/dependent|implication/i); diff --git a/test/modules/candidatePostProcessor.test.ts b/test/modules/candidatePostProcessor.test.ts index 228f86738..8e6c196db 100644 --- a/test/modules/candidatePostProcessor.test.ts +++ b/test/modules/candidatePostProcessor.test.ts @@ -169,6 +169,7 @@ describe('dirLetterLimit', () => { commands: ['copy'], output: 'Output', dirLetter: true, + dirLetterCount: 1, dirLetterLimit: limit, dirGameSubdir: GameSubdirMode[GameSubdirMode.MULTIPLE].toLowerCase(), }); diff --git a/test/outputFactory.test.ts b/test/outputFactory.test.ts index f38138bc2..4ec0ed3cd 100644 --- a/test/outputFactory.test.ts +++ b/test/outputFactory.test.ts @@ -889,11 +889,26 @@ describe('should respect "--dir-dat-description"', () => { describe('should respect "--dir-letter"', () => { describe('games with one ROM', () => { test.each([ - ['', os.devNull], - ['file.rom', path.join(os.devNull, 'F', 'file.rom')], - ['🙂.rom', path.join(os.devNull, '#', '🙂.rom')], - ])('option is true: %s', async (romName, expectedPath) => { - const options = new Options({ commands: ['copy'], output: os.devNull, dirLetter: true }); + [0, '', os.devNull], + [1, '', os.devNull], + [2, '', os.devNull], + [999, '', os.devNull], + [1, 'file.rom', path.join(os.devNull, 'F', 'file.rom')], + [3, 'file.rom', path.join(os.devNull, 'FIL', 'file.rom')], + [10, 'file.rom', path.join(os.devNull, 'FILEAAAAAA', 'file.rom')], + [1, '007.rom', path.join(os.devNull, '#', '007.rom')], + [2, '007.rom', path.join(os.devNull, '##', '007.rom')], + [10, '007.rom', path.join(os.devNull, '###AAAAAAA', '007.rom')], + [1, '🙂.rom', path.join(os.devNull, '#', '🙂.rom')], + [3, '🙂.rom', path.join(os.devNull, '##A', '🙂.rom')], + [10, '🙂.rom', path.join(os.devNull, '##AAAAAAAA', '🙂.rom')], + ])('option is true: %s', async (dirLetterCount, romName, expectedPath) => { + const options = new Options({ + commands: ['copy'], + output: os.devNull, + dirLetter: true, + dirLetterCount, + }); const rom = new ROM({ name: romName, size: 0, crc: '' }); const outputPath = OutputFactory.getPath( @@ -939,6 +954,7 @@ describe('should respect "--dir-letter"', () => { commands: ['copy'], output: os.devNull, dirLetter: true, + dirLetterCount: 1, dirGameSubdir: GameSubdirMode[GameSubdirMode.MULTIPLE].toLowerCase(), }); From 321f8c8276ab798b4708e569f54d9db86ade742d Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:02:42 -0800 Subject: [PATCH 2/8] Comments --- src/types/outputFactory.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types/outputFactory.ts b/src/types/outputFactory.ts index 09dd8798d..13b834352 100644 --- a/src/types/outputFactory.ts +++ b/src/types/outputFactory.ts @@ -334,7 +334,7 @@ export default class OutputFactory { .padEnd(options.getDirLetterCount(), 'A') .toUpperCase() .replace(/[^A-Z0-9]/g, '#'); - // TODO(cemmer): only do this when not grouping letters together + // TODO(cemmer): only do this when not --dir-letter-group letters = letters.replace(/[^A-Z]/g, '#'); const existing = map.get(letters) ?? new Set(); @@ -346,7 +346,7 @@ export default class OutputFactory { [...lettersToFilenames.entries()] .sort((a, b) => a[0].localeCompare(b[0])) .reduce((arr, [letter, filenames]) => { - // TODO: group letters together + // TODO(cemmer): --dir-letter-group const tuples = [...filenames.values()] .sort() .map((filename) => [letter, filename] satisfies [string, string]); From 7acb7dadc75c927d7a744ef007eabc13234cc6cd Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:13:26 -0800 Subject: [PATCH 3/8] Update --- src/modules/argumentsParser.ts | 6 +++--- test/modules/argumentsParser.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index e1e5517e6..e8e5a5225 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -295,7 +295,7 @@ export default class ArgumentsParser { }) .option('dir-letter', { group: groupRomOutput, - description: 'Group games in an output subdirectory by the first letters in their name', + description: 'Group games in an output subdirectory by the first --dir-letter-count letters in their name', type: 'boolean', }) .option('dir-letter-count', { @@ -309,7 +309,7 @@ export default class ArgumentsParser { }) .option('dir-letter-limit', { group: groupRomOutput, - description: 'Limit the number ROMs in letter subdirectories, splitting into multiple if necessary', + description: 'Limit the number games in letter subdirectories, splitting into multiple subdirectories if necessary', type: 'number', coerce: (val: number) => Math.max(ArgumentsParser.getLastValue(val), 1), requiresArg: true, @@ -317,7 +317,7 @@ export default class ArgumentsParser { }) .option('dir-game-subdir', { group: groupRomOutput, - description: 'Append the name of the game as an output directory depending on its ROMs', + description: 'Append the name of the game as an output subdirectory depending on its ROMs', choices: Object.keys(GameSubdirMode) .filter((mode) => Number.isNaN(Number(mode))) .map((mode) => mode.toLowerCase()), diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts index b2c62c9fc..aab663b2a 100644 --- a/test/modules/argumentsParser.test.ts +++ b/test/modules/argumentsParser.test.ts @@ -111,8 +111,8 @@ describe('options', () => { expect(options.getDirDatName()).toEqual(false); expect(options.getDirDatDescription()).toEqual(false); expect(options.getDirLetter()).toEqual(false); - expect(options.getDirLetterLimit()).toEqual(0); expect(options.getDirLetterCount()).toEqual(1); + expect(options.getDirLetterLimit()).toEqual(0); expect(options.getDirGameSubdir()).toEqual(GameSubdirMode.MULTIPLE); expect(options.getOverwrite()).toEqual(false); expect(options.getOverwriteInvalid()).toEqual(false); From 3a2878c4513c8f39383524b7ca6dfce2ef9412ab Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:21:22 -0800 Subject: [PATCH 4/8] Remove --- src/types/outputFactory.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/types/outputFactory.ts b/src/types/outputFactory.ts index 13b834352..59e1f3706 100644 --- a/src/types/outputFactory.ts +++ b/src/types/outputFactory.ts @@ -343,16 +343,6 @@ export default class OutputFactory { return map; }, new Map>()); - [...lettersToFilenames.entries()] - .sort((a, b) => a[0].localeCompare(b[0])) - .reduce((arr, [letter, filenames]) => { - // TODO(cemmer): --dir-letter-group - const tuples = [...filenames.values()] - .sort() - .map((filename) => [letter, filename] satisfies [string, string]); - return [...arr, ...tuples]; - }, [] as [string, string][]); - // Split the letter directories, if needed if (options.getDirLetterLimit()) { lettersToFilenames = [...lettersToFilenames.entries()] From ff139f356df2b18303ff819c5cfc6f950e3cbb82 Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Tue, 30 Jan 2024 16:59:15 -0800 Subject: [PATCH 5/8] Update src/modules/argumentsParser.ts Co-authored-by: Sam Cole --- src/modules/argumentsParser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index e8e5a5225..7ab6e81ee 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -309,7 +309,7 @@ export default class ArgumentsParser { }) .option('dir-letter-limit', { group: groupRomOutput, - description: 'Limit the number games in letter subdirectories, splitting into multiple subdirectories if necessary', + description: 'Limit the number of games in letter subdirectories, splitting into multiple subdirectories if necessary', type: 'number', coerce: (val: number) => Math.max(ArgumentsParser.getLastValue(val), 1), requiresArg: true, From c03c1e09f6e71726f588bc80ffaa4a0f84ec656f Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Tue, 30 Jan 2024 20:01:12 -0800 Subject: [PATCH 6/8] Dependent options --- src/modules/argumentsParser.ts | 8 +++++++- test/modules/argumentsParser.test.ts | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index 7ab6e81ee..059f925f4 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -304,9 +304,15 @@ export default class ArgumentsParser { type: 'number', coerce: (val: number) => Math.max(ArgumentsParser.getLastValue(val), 1), requiresArg: true, - // Note: can't `implies: 'dir-letter'` with a default value set default: 1, }) + .check((checkArgv) => { + // Re-implement `implies: 'dir-letter'`, which isn't possible with a default value + if (checkArgv['dir-letter-count'] && !checkArgv['dir-letter']) { + throw new Error('Missing dependent arguments: dir-letter-count -> dir-letter'); + } + return true; + }) .option('dir-letter-limit', { group: groupRomOutput, description: 'Limit the number of games in letter subdirectories, splitting into multiple subdirectories if necessary', diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts index aab663b2a..e72093d95 100644 --- a/test/modules/argumentsParser.test.ts +++ b/test/modules/argumentsParser.test.ts @@ -348,6 +348,8 @@ describe('options', () => { }); it('should parse "dir-letter-count"', () => { + expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count'])).toThrow(/not enough arguments/i); + expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count', '1'])).toThrow(/dependent|implication/i); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '-1']).getDirLetterCount()).toEqual(1); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '0']).getDirLetterCount()).toEqual(1); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '1']).getDirLetterCount()).toEqual(1); From bd0f16de4002c4a3e95d74a0cdabdede57fb08ab Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Tue, 30 Jan 2024 20:17:10 -0800 Subject: [PATCH 7/8] Newline --- src/modules/argumentsParser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index 059f925f4..39ec3415f 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -309,7 +309,7 @@ export default class ArgumentsParser { .check((checkArgv) => { // Re-implement `implies: 'dir-letter'`, which isn't possible with a default value if (checkArgv['dir-letter-count'] && !checkArgv['dir-letter']) { - throw new Error('Missing dependent arguments: dir-letter-count -> dir-letter'); + throw new Error('Missing dependent arguments:\n dir-letter-count -> dir-letter'); } return true; }) From d86cbcd9ab0924cb59e7ba797ca8222d1643e2c5 Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Tue, 30 Jan 2024 21:25:28 -0800 Subject: [PATCH 8/8] Validation fix --- src/modules/argumentsParser.ts | 2 +- test/modules/argumentsParser.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/argumentsParser.ts b/src/modules/argumentsParser.ts index 39ec3415f..15206b786 100644 --- a/src/modules/argumentsParser.ts +++ b/src/modules/argumentsParser.ts @@ -308,7 +308,7 @@ export default class ArgumentsParser { }) .check((checkArgv) => { // Re-implement `implies: 'dir-letter'`, which isn't possible with a default value - if (checkArgv['dir-letter-count'] && !checkArgv['dir-letter']) { + if (checkArgv['dir-letter-count'] > 1 && !checkArgv['dir-letter']) { throw new Error('Missing dependent arguments:\n dir-letter-count -> dir-letter'); } return true; diff --git a/test/modules/argumentsParser.test.ts b/test/modules/argumentsParser.test.ts index e72093d95..f92bd28f1 100644 --- a/test/modules/argumentsParser.test.ts +++ b/test/modules/argumentsParser.test.ts @@ -349,7 +349,8 @@ describe('options', () => { it('should parse "dir-letter-count"', () => { expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count'])).toThrow(/not enough arguments/i); - expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count', '1'])).toThrow(/dependent|implication/i); + expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count', '1'])).not.toThrow(); + expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter-count', '2'])).toThrow(/dependent|implication/i); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '-1']).getDirLetterCount()).toEqual(1); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '0']).getDirLetterCount()).toEqual(1); expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dir-letter', '--dir-letter-count', '1']).getDirLetterCount()).toEqual(1);