Merge pull request #142 from cakephp/cleanup-input-option

Follow up to #137
cakephp · Oct 16, 2015 · 8fd5d9f · 8fd5d9f
2 parents a0b5b62 + befbda1
commit 8fd5d9f
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 44 deletions.
diff --git a/src/Command/Create.php b/src/Command/Create.php
@@ -34,9 +34,9 @@ protected function configure()
                 PHP_EOL,
                 PHP_EOL
             ));
-        $this->addOption('plugin', 'p', InputArgument::OPTIONAL, 'The plugin the file should be created for')
-            ->addOption('connection', 'c', InputArgument::OPTIONAL, 'The datasource connection to use')
-            ->addOption('source', 's', InputArgument::OPTIONAL, 'The folder where migrations are in')
+        $this->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for')
+            ->addOption('connection', 'c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')
+            ->addOption('source', 's', InputOption::VALUE_REQUIRED, 'The folder where migrations are in')
             ->addOption('template', 't', InputOption::VALUE_REQUIRED, 'Use an alternative template')
             ->addOption(
                 'class',

diff --git a/src/Command/MarkMigrated.php b/src/Command/MarkMigrated.php
@@ -11,6 +11,7 @@
  */
 namespace Migrations\Command;
 
+use InvalidArgumentException;
 use Migrations\ConfigurationTrait;
 use Phinx\Console\Command\AbstractCommand;
 use Symfony\Component\Console\Input\InputArgument;
@@ -52,32 +53,32 @@ protected function configure()
             ->addArgument(
                 'version',
                 InputArgument::OPTIONAL,
-                'DEPRECATED: use `bin/cake migrations mark_migrated --target=VERSION` instead'
+                'DEPRECATED: use `bin/cake migrations mark_migrated --target=VERSION --only` instead'
             )
             ->setHelp(sprintf(
                 '%sMark migrations as migrated%s',
                 PHP_EOL,
                 PHP_EOL
-            ));
-        $this->addOption('plugin', 'p', InputArgument::OPTIONAL, 'The plugin the file should be created for')
-            ->addOption('connection', 'c', InputArgument::OPTIONAL, 'The datasource connection to use')
-            ->addOption('source', 's', InputArgument::OPTIONAL, 'The folder where migrations are in')
+            ))
+            ->addOption('plugin', 'p', InputOption::VALUE_REQUIRED, 'The plugin the file should be created for')
+            ->addOption('connection', 'c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')
+            ->addOption('source', 's', InputOption::VALUE_REQUIRED, 'The folder where migrations are in')
             ->addOption(
                 'target',
                 't',
-                InputArgument::OPTIONAL,
+                InputOption::VALUE_REQUIRED,
                 'It will mark migrations from beginning to the given version'
             )
             ->addOption(
                 'exclude',
                 'x',
-                InputArgument::OPTIONAL,
+                InputOption::VALUE_NONE,
                 'If present it will mark migrations from beginning until the given version, excluding it'
             )
             ->addOption(
                 'only',
                 'o',
-                InputArgument::OPTIONAL,
+                InputOption::VALUE_NONE,
                 'If present it will only mark the given migration version'
             );
     }
@@ -122,7 +123,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         try {
             $versions = $this->getVersionsToMark($input);
-        } catch (\InvalidArgumentException $e) {
+        } catch (InvalidArgumentException $e) {
             $output->writeln(sprintf("<error>%s</error>", $e->getMessage()));
             return;
         }
@@ -181,7 +182,7 @@ protected function markVersionsAsMigrated($path, $versions)
      * Decides which versions it should mark as migrated
      *
      * @return array Array of versions that should be marked as migrated
-     * @throws InvalidArgumentException If the `--exclude` or `--only` options are used without `--target`
+     * @throws \InvalidArgumentException If the `--exclude` or `--only` options are used without `--target`
      * or version not found
      */
     protected function getVersionsToMark()
@@ -197,7 +198,7 @@ protected function getVersionsToMark()
 
         if ($this->isOnly()) {
             if (!in_array($version, $versions)) {
-                throw new \InvalidArgumentException("Migration `$version` was not found !");
+                throw new InvalidArgumentException("Migration `$version` was not found !");
             }
 
             return [$version];
@@ -267,7 +268,7 @@ protected function isOnly()
      */
     protected function hasExclude()
     {
-        return $this->input->getOption('exclude') !== null;
+        return $this->input->getOption('exclude');
     }
 
     /**
@@ -277,7 +278,7 @@ protected function hasExclude()
      */
     protected function hasOnly()
     {
-        return $this->input->getOption('only') !== null;
+        return $this->input->getOption('only');
     }
 
     /**

diff --git a/src/Command/Migrate.php b/src/Command/Migrate.php
@@ -16,6 +16,7 @@
 use Phinx\Console\Command\Migrate as MigrateCommand;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class Migrate extends MigrateCommand
@@ -34,11 +35,11 @@ protected function configure()
         $this->setName('migrate')
             ->setDescription('Migrate the database')
             ->setHelp('runs all available migrations, optionally up to a specific version')
-            ->addOption('--target', '-t', InputArgument::OPTIONAL, 'The version number to migrate to')
-            ->addOption('--date', '-d', InputArgument::OPTIONAL, 'The date to migrate to')
-            ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations')
-            ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use')
-            ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in');
+            ->addOption('--target', '-t', InputOption::VALUE_REQUIRED, 'The version number to migrate to')
+            ->addOption('--date', '-d', InputOption::VALUE_REQUIRED, 'The date to migrate to')
+            ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations')
+            ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')
+            ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in');
     }
 
     /**

diff --git a/src/Command/Rollback.php b/src/Command/Rollback.php
@@ -16,6 +16,7 @@
 use Phinx\Console\Command\Rollback as RollbackCommand;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class Rollback extends RollbackCommand
@@ -34,11 +35,11 @@ protected function configure()
         $this->setName('rollback')
             ->setDescription('Rollback the last or to a specific migration')
             ->setHelp('reverts the last migration, or optionally up to a specific version')
-            ->addOption('--target', '-t', InputArgument::OPTIONAL, 'The version number to rollback to')
-            ->addOption('--date', '-d', InputArgument::OPTIONAL, 'The date to migrate to')
-            ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations')
-            ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use')
-            ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in');
+            ->addOption('--target', '-t', InputOption::VALUE_REQUIRED, 'The version number to rollback to')
+            ->addOption('--date', '-d', InputOption::VALUE_REQUIRED, 'The date to migrate to')
+            ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations')
+            ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')
+            ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in');
     }
 
     /**

diff --git a/src/Command/Status.php b/src/Command/Status.php
@@ -15,6 +15,7 @@
 use Phinx\Console\Command\Status as StatusCommand;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class Status extends StatusCommand
@@ -29,11 +30,16 @@ protected function configure()
     {
         $this->setName('status')
             ->setDescription('Show migration status')
-            ->addOption('--format', '-f', InputArgument::OPTIONAL, 'The output format: text or json. Defaults to text.')
+            ->addOption(
+                '--format',
+                '-f',
+                InputOption::VALUE_REQUIRED,
+                'The output format: text or json. Defaults to text.'
+            )
             ->setHelp('prints a list of all migrations, along with their current status')
-            ->addOption('--plugin', '-p', InputArgument::OPTIONAL, 'The plugin containing the migrations')
-            ->addOption('--connection', '-c', InputArgument::OPTIONAL, 'The datasource connection to use')
-            ->addOption('--source', '-s', InputArgument::OPTIONAL, 'The folder where migrations are in');
+            ->addOption('--plugin', '-p', InputOption::VALUE_REQUIRED, 'The plugin containing the migrations')
+            ->addOption('--connection', '-c', InputOption::VALUE_REQUIRED, 'The datasource connection to use')
+            ->addOption('--source', '-s', InputOption::VALUE_REQUIRED, 'The folder where migrations are in');
     }
 
     /**

diff --git a/src/Shell/MigrationsShell.php b/src/Shell/MigrationsShell.php
@@ -49,7 +49,9 @@ public function getOptionParser()
             ->addOption('version', ['short' => 'V'])
             ->addOption('no-interaction', ['short' => 'n'])
             ->addOption('template', ['short' => 't'])
-            ->addOption('format', ['short' => 'f']);
+            ->addOption('format', ['short' => 'f'])
+            ->addOption('only', ['short' => 'o'])
+            ->addOption('exclude', ['short' => 'x']);
     }
 
     /**

diff --git a/src/Shell/Task/MigrationSnapshotTask.php b/src/Shell/Task/MigrationSnapshotTask.php
@@ -85,7 +85,7 @@ protected function markSnapshotApplied($path)
         list($version, ) = explode('_', $fileName, 2);
 
 
-        $dispatchCommand = 'migrations mark_migrated ' . $version;
+        $dispatchCommand = 'migrations mark_migrated -t ' . $version . ' -o';
         if (!empty($this->params['connection'])) {
             $dispatchCommand .= ' -c ' . $this->params['connection'];
         }

diff --git a/tests/TestCase/Command/MarkMigratedTest.php b/tests/TestCase/Command/MarkMigratedTest.php
@@ -293,7 +293,7 @@ public function testExecuteTargetWithExclude()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150724233100',
-            '--exclude' => '',
+            '--exclude' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -309,7 +309,7 @@ public function testExecuteTargetWithExclude()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150826191400',
-            '--exclude' => '',
+            '--exclude' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -332,7 +332,7 @@ public function testExecuteTargetWithExclude()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150704160610',
-            '--exclude' => '',
+            '--exclude' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -348,7 +348,7 @@ public function testExecuteTargetWithOnly()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150724233100',
-            '--only' => '',
+            '--only' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -364,7 +364,7 @@ public function testExecuteTargetWithOnly()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150826191400',
-            '--only' => '',
+            '--only' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -383,7 +383,7 @@ public function testExecuteTargetWithOnly()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150704160610',
-            '--only' => '',
+            '--only' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -422,7 +422,7 @@ public function testExecuteInvalidUseOfOnlyAndExclude()
     {
         $this->commandTester->execute([
             'command' => $this->command->getName(),
-            '--exclude' => '',
+            '--exclude' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -436,7 +436,7 @@ public function testExecuteInvalidUseOfOnlyAndExclude()
 
         $this->commandTester->execute([
             'command' => $this->command->getName(),
-            '--only' => '',
+            '--only' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);
@@ -451,8 +451,8 @@ public function testExecuteInvalidUseOfOnlyAndExclude()
         $this->commandTester->execute([
             'command' => $this->command->getName(),
             '--target' => '20150724233100',
-            '--only' => '',
-            '--exclude' => '',
+            '--only' => true,
+            '--exclude' => true,
             '--connection' => 'test',
             '--source' => 'TestsMigrations'
         ]);

diff --git a/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php b/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php
@@ -81,8 +81,8 @@ public function testNotEmptySnapshot()
             ->method('dispatchShell')
             ->with(
                 $this->logicalAnd(
-                    $this->stringContains('migrations mark_migrated'),
-                    $this->stringContains('-c test -p BogusPlugin')
+                    $this->stringContains('migrations mark_migrated -t'),
+                    $this->stringContains('-o -c test -p BogusPlugin')
                 )
             );