Fix: fixdat command rather than option (#566)

docs/input/dats.md

@@ -169,14 +169,14 @@ See the [arcade page](../usage/arcade.md) for more information on building & re-
 
 Fixdats help you find files missing from your collection, and they can be used to generate a collection of those files once you've found them. This sub-collection of files can then be merged back into your main collection.
 
-The `--fixdat` option creates a [Logiqx XML](http://www.logiqx.com/DatFAQs/) DAT for every input DAT (the `--dat <path>` option) that is missing ROMs. When writing (`copy`, `move`, and `symlink` commands), the fixdat will be written to the output directory, otherwise it will be written to the working directory.
+The `fixdat` command creates a [Logiqx XML](http://www.logiqx.com/DatFAQs/) DAT for every input DAT (the `--dat <path>` option) that is missing ROMs. When writing (`copy`, `move`, and `symlink` commands), the fixdat will be written to the output directory, otherwise it will be written to the working directory.
 
 For example:
 
 === ":simple-windowsxp: Windows"
 
     ```batch
-    igir copy zip ^
+    igir copy zip fixdat ^
       --dat "Nintendo - Game Boy.dat" ^
       --dat "Nintendo - Game Boy Advance.dat" ^
       --dat "Nintendo - Game Boy Color.dat" ^
@@ -188,25 +188,23 @@ For example:
 === ":simple-apple: macOS"
 
     ```shell
-    igir copy zip \
+    igir copy zip fixdat \
       --dat "Nintendo - Game Boy.dat" \
       --dat "Nintendo - Game Boy Advance.dat" \
       --dat "Nintendo - Game Boy Color.dat" \
       --input ROMs/ \
-      --output ROMs-Sorted/ \
-      --fixdat
+      --output ROMs-Sorted/
     ```
 
 === ":simple-linux: Linux"
 
     ```shell
-    igir copy zip \
+    igir copy zip fixdat \
       --dat "Nintendo - Game Boy.dat" \
       --dat "Nintendo - Game Boy Advance.dat" \
       --dat "Nintendo - Game Boy Color.dat" \
       --input ROMs/ \
-      --output ROMs-Sorted/ \
-      --fixdat
+      --output ROMs-Sorted/
     ```
 
 may produce some fixdats in the `ROMs-Sorted/` directory, if any of the input DATs have ROMs that weren't found in the `ROMs/` input directory:

docs/internals.md

@@ -17,7 +17,7 @@ Information about the inner workings of `igir`.
    - ROM preferences are applied (`--single`, see [filtering & preference docs](roms/filtering-preferences.md))
    - ROMs are written to the output directory, if specified (`copy`, `move`, `symlink`)
      - Written ROMs are tested for accuracy, if specified (`test`)
-   - A "fixdat" is created, if specified (`--fixdat`)
+   - A "fixdat" is created, if specified (`fixdat`)
 5. "Moved" input ROMs are deleted (`move`)
 6. Unknown files are recycled from the output directory, if specified (`clean`, see [cleaning docs](output/cleaning.md))
 7. An output report is written to the output directory, if specified (`report`, see [reporting docs](output/reporting.md))

src/modules/argumentsParser.ts

@@ -55,7 +55,6 @@ export default class ArgumentsParser {
 
     const groupInput = 'Input options (supports globbing):';
     const groupDatInput = 'DAT input options:';
-    const groupDatOutput = 'DAT output options:';
     const groupRomOutput = 'ROM output options:';
     const groupRomZip = 'ROM zip command options:';
     const groupRomSymlink = 'ROM symlink command options:';
@@ -87,6 +86,9 @@ export default class ArgumentsParser {
       .command('test', 'Test ROMs for accuracy after writing them to the output directory', (yargsSubObj) => {
         addCommands(yargsSubObj);
       })
+      .command('fixdat', 'Generate a fixdat of any missing games for every DAT processed (requires --dat)', (yargsSubObj) => {
+        addCommands(yargsSubObj);
+      })
       .command('clean', 'Recycle unknown files in the output directory', (yargsSubObj) => {
         addCommands(yargsSubObj);
       })
@@ -207,10 +209,14 @@ export default class ArgumentsParser {
       })
 
       .option('fixdat', {
-        group: groupDatOutput,
-        description: 'Generate a fixdat of any missing games for every DAT processed (requires --dat)',
         type: 'boolean',
+        coerce: (val: boolean) => {
+          this.logger.warn('--fixdat is deprecated, use the fixdat command instead');
+          return val;
+        },
         implies: 'dat',
+        deprecated: true,
+        hidden: true,
       })
 
       .option('output', {

src/modules/fixdatCreator.ts

@@ -34,7 +34,7 @@ export default class FixdatCreator extends Module {
     originalDat: DAT,
     parentsToCandidates: Map<Parent, ReleaseCandidate[]>,
   ): Promise<string | undefined> {
-    if (!this.options.getFixdat()) {
+    if (!this.options.shouldFixdat()) {
       return undefined;
     }
 

src/types/options.ts

@@ -466,10 +466,10 @@ export default class Options implements OptionsProps {
   }
 
   /**
-   * Was the `clean` command provided?
+   * Was the 'fixdat' command provided?
    */
-  shouldClean(): boolean {
-    return this.getCommands().has('clean');
+  shouldFixdat(): boolean {
+    return this.getCommands().has('fixdat') || this.fixdat;
   }
 
   /**
@@ -479,6 +479,13 @@ export default class Options implements OptionsProps {
     return this.getCommands().has('test');
   }
 
+  /**
+   * Was the `clean` command provided?
+   */
+  shouldClean(): boolean {
+    return this.getCommands().has('clean');
+  }
+
   /**
    * Was the `report` command provided?
    */
@@ -661,10 +668,6 @@ export default class Options implements OptionsProps {
     return Options.getRegex(this.datRegexExclude);
   }
 
-  getFixdat(): boolean {
-    return this.fixdat;
-  }
-
   getOutput(): string {
     return this.shouldWrite() ? this.output : Constants.GLOBAL_TEMP_DIR;
   }

test/modules/argumentsParser.test.ts

@@ -41,50 +41,43 @@ describe('commands', () => {
     expect(() => argumentsParser.parse(['clean', ...dummyRequiredArgs])).toThrow(/command.+requires/i);
   });
 
-  it('should not parse different commands', () => {
+  it('should not parse commands not present', () => {
     expect(argumentsParser.parse(['move', ...dummyRequiredArgs]).shouldCopy()).toEqual(false);
     expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldMove()).toEqual(false);
     expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldExtract()).toEqual(false);
     expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldZip('')).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldClean()).toEqual(false);
     expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldTest()).toEqual(false);
+    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldFixdat()).toEqual(false);
+    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldClean()).toEqual(false);
     expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldReport()).toEqual(false);
   });
 
   it('should parse multiple commands', () => {
-    const copyExtract = ['copy', 'extract', 'clean', 'test', 'report', ...dummyRequiredArgs, '--dat', os.devNull];
+    const copyExtract = ['copy', 'extract', 'test', 'fixdat', 'clean', 'report', ...dummyRequiredArgs, '--dat', os.devNull];
     expect(argumentsParser.parse(copyExtract).shouldCopy()).toEqual(true);
     expect(argumentsParser.parse(copyExtract).shouldMove()).toEqual(false);
     expect(argumentsParser.parse(copyExtract).shouldExtract()).toEqual(true);
     expect(argumentsParser.parse(copyExtract).shouldZip('')).toEqual(false);
-    expect(argumentsParser.parse(copyExtract).shouldClean()).toEqual(true);
     expect(argumentsParser.parse(copyExtract).shouldTest()).toEqual(true);
+    expect(argumentsParser.parse(copyExtract).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse(copyExtract).shouldClean()).toEqual(true);
     expect(argumentsParser.parse(copyExtract).shouldReport()).toEqual(true);
 
-    const moveZip = ['move', 'zip', 'clean', 'test', 'report', ...dummyRequiredArgs, '--dat', os.devNull];
+    const moveZip = ['move', 'zip', 'test', 'fixdat', 'clean', 'report', ...dummyRequiredArgs, '--dat', os.devNull];
     expect(argumentsParser.parse(moveZip).shouldCopy()).toEqual(false);
     expect(argumentsParser.parse(moveZip).shouldMove()).toEqual(true);
     expect(argumentsParser.parse(moveZip).shouldExtract()).toEqual(false);
     expect(argumentsParser.parse(moveZip).shouldZip('')).toEqual(true);
-    expect(argumentsParser.parse(moveZip).shouldClean()).toEqual(true);
     expect(argumentsParser.parse(moveZip).shouldTest()).toEqual(true);
+    expect(argumentsParser.parse(moveZip).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse(moveZip).shouldClean()).toEqual(true);
     expect(argumentsParser.parse(moveZip).shouldReport()).toEqual(true);
   });
 
   it('should parse duplicate commands', () => {
     expect(argumentsParser.parse(['copy', 'copy', 'copy', ...dummyRequiredArgs]).shouldCopy()).toEqual(true);
   });
 
-  it('should not parse commands not present', () => {
-    expect(argumentsParser.parse(['move', ...dummyRequiredArgs]).shouldCopy()).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldMove()).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldExtract()).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldZip('')).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldClean()).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldTest()).toEqual(false);
-    expect(argumentsParser.parse(['copy', ...dummyRequiredArgs]).shouldReport()).toEqual(false);
-  });
-
   it('should throw on unrecognized options', () => {
     expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '-q'])).toThrow(/unknown argument/i);
     expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--quiet'])).toThrow(/unknown argument/i);
@@ -95,7 +88,15 @@ describe('options', () => {
   it('should have expected defaults', () => {
     const options = argumentsParser.parse(dummyCommandAndRequiredArgs);
 
-    expect(options.getFixdat()).toEqual(false);
+    expect(options.shouldCopy()).toEqual(true); // dummy command
+    expect(options.shouldMove()).toEqual(false);
+    expect(options.shouldSymlink()).toEqual(false);
+    expect(options.shouldExtract()).toEqual(false);
+    expect(options.canZip()).toEqual(false);
+    expect(options.shouldFixdat()).toEqual(false);
+    expect(options.shouldTest()).toEqual(false);
+    expect(options.shouldClean()).toEqual(false);
+    expect(options.shouldReport()).toEqual(false);
 
     expect(options.getDirMirror()).toEqual(false);
     expect(options.getDirDatName()).toEqual(false);
@@ -246,12 +247,12 @@ describe('options', () => {
   });
 
   it('should parse "fixdat"', () => {
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat']).getFixdat()).toEqual(true);
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'true']).getFixdat()).toEqual(true);
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'false']).getFixdat()).toEqual(false);
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', '--fixdat']).getFixdat()).toEqual(true);
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'false', '--fixdat', 'true']).getFixdat()).toEqual(true);
-    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'true', '--fixdat', 'false']).getFixdat()).toEqual(false);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat']).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'true']).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'false']).shouldFixdat()).toEqual(false);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', '--fixdat']).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'false', '--fixdat', 'true']).shouldFixdat()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull, '--fixdat', 'true', '--fixdat', 'false']).shouldFixdat()).toEqual(false);
   });
 
   it('should parse "output"', () => {