Minor bin/aphlict cleanup

Summary: Ref T10697. This just improves a couple of minor `bin/aphlict` things: make argument parsing more explicit/consistent, consolidate a little bit of duplicated code.

Test Plan: Ran all `bin/aphlict` commands.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10697

Differential Revision: https://secure.phabricator.com/D15698

src/applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php

@@ -4,17 +4,18 @@ final class PhabricatorAphlictManagementDebugWorkflow
   extends PhabricatorAphlictManagementWorkflow {
 
   protected function didConstruct() {
-    parent::didConstruct();
     $this
       ->setName('debug')
       ->setSynopsis(
         pht(
           'Start the notifications server in the foreground and print large '.
-          'volumes of diagnostic information to the console.'));
+          'volumes of diagnostic information to the console.'))
+      ->setArguments($this->getLaunchArguments());
   }
 
   public function execute(PhutilArgumentParser $args) {
-    parent::execute($args);
+    $this->parseLaunchArguments($args);
+
     $this->setDebug(true);
 
     $this->willLaunch();

src/applications/aphlict/management/PhabricatorAphlictManagementRestartWorkflow.php

@@ -4,19 +4,20 @@ final class PhabricatorAphlictManagementRestartWorkflow
   extends PhabricatorAphlictManagementWorkflow {
 
   protected function didConstruct() {
-    parent::didConstruct();
     $this
       ->setName('restart')
-      ->setSynopsis(pht('Stop, then start the notifications server.'));
+      ->setSynopsis(pht('Stop, then start the notification server.'))
+      ->setArguments($this->getLaunchArguments());
   }
 
   public function execute(PhutilArgumentParser $args) {
-    parent::execute($args);
+    $this->parseLaunchArguments($args);
 
     $err = $this->executeStopCommand();
     if ($err) {
       return $err;
     }
+
     return $this->executeStartCommand();
   }
 

src/applications/aphlict/management/PhabricatorAphlictManagementStartWorkflow.php

@@ -4,14 +4,14 @@ final class PhabricatorAphlictManagementStartWorkflow
   extends PhabricatorAphlictManagementWorkflow {
 
   protected function didConstruct() {
-    parent::didConstruct();
     $this
       ->setName('start')
-      ->setSynopsis(pht('Start the notifications server.'));
+      ->setSynopsis(pht('Start the notifications server.'))
+      ->setArguments($this->getLaunchArguments());
   }
 
   public function execute(PhutilArgumentParser $args) {
-    parent::execute($args);
+    $this->parseLaunchArguments($args);
     return $this->executeStartCommand();
   }
 

src/applications/aphlict/management/PhabricatorAphlictManagementStatusWorkflow.php

@@ -6,7 +6,7 @@ final class PhabricatorAphlictManagementStatusWorkflow
   protected function didConstruct() {
     $this
       ->setName('status')
-      ->setSynopsis(pht('Show the status of the notifications server.'))
+      ->setSynopsis(pht('Show the status of the notification server.'))
       ->setArguments(array());
   }
 

src/applications/aphlict/management/PhabricatorAphlictManagementStopWorkflow.php

@@ -6,7 +6,7 @@ final class PhabricatorAphlictManagementStopWorkflow
   protected function didConstruct() {
     $this
       ->setName('stop')
-      ->setSynopsis(pht('Stop the notifications server.'))
+      ->setSynopsis(pht('Stop the notification server.'))
       ->setArguments(array());
   }
 

src/applications/aphlict/management/PhabricatorAphlictManagementWorkflow.php

@@ -7,27 +7,29 @@ abstract class PhabricatorAphlictManagementWorkflow
   private $clientHost;
   private $clientPort;
 
-  protected function didConstruct() {
+  final protected function setDebug($debug) {
-    $this
+    $this->debug = $debug;
-      ->setArguments(
+    return $this;
-        array(
-          array(
-            'name'  => 'client-host',
-            'param' => 'hostname',
-            'help'  => pht('Hostname to bind to for the client server.'),
-          ),
-          array(
-            'name'  => 'client-port',
-            'param' => 'port',
-            'help'  => pht('Port to bind to for the client server.'),
-          ),
-        ));
   }
 
-  public function execute(PhutilArgumentParser $args) {
+  protected function getLaunchArguments() {
+    return array(
+      array(
+        'name'  => 'client-host',
+        'param' => 'hostname',
+        'help'  => pht('Hostname to bind to for the client server.'),
+      ),
+      array(
+        'name'  => 'client-port',
+        'param' => 'port',
+        'help'  => pht('Port to bind to for the client server.'),
+      ),
+    );
+  }
+
+  protected function parseLaunchArguments(PhutilArgumentParser $args) {
     $this->clientHost = $args->getArg('client-host');
     $this->clientPort = $args->getArg('client-port');
-    return 0;
   }
 
   final public function getPIDPath() {
@@ -86,11 +88,6 @@ abstract class PhabricatorAphlictManagementWorkflow
     exit(1);
   }
 
-  final protected function setDebug($debug) {
-    $this->debug = $debug;
-    return $this;
-  }
-
   public static function requireExtensions() {
     self::mustHaveExtension('pcntl');
     self::mustHaveExtension('posix');
@@ -146,11 +143,8 @@ abstract class PhabricatorAphlictManagementWorkflow
     $test_argv = $this->getServerArgv();
     $test_argv[] = '--test=true';
 
-    execx(
+
-      '%s %s %Ls',
+    execx('%C', $this->getStartCommand($test_argv));
-      $this->getNodeBinary(),
-      $this->getAphlictScriptPath(),
-      $test_argv);
   }
 
   private function getServerArgv() {
@@ -189,11 +183,6 @@ abstract class PhabricatorAphlictManagementWorkflow
     return $server_argv;
   }
 
-  private function getAphlictScriptPath() {
-    $root = dirname(phutil_get_library_root('phabricator'));
-    return $root.'/support/aphlict/server/aphlict_server.js';
-  }
-
   final protected function launch() {
     $console = PhutilConsole::getConsole();
 
@@ -205,11 +194,7 @@ abstract class PhabricatorAphlictManagementWorkflow
       Filesystem::writeFile($this->getPIDPath(), getmypid());
     }
 
-    $command = csprintf(
+    $command = $this->getStartCommand($this->getServerArgv());
-      '%s %s %Ls',
-      $this->getNodeBinary(),
-      $this->getAphlictScriptPath(),
-      $this->getServerArgv());
 
     if (!$this->debug) {
       declare(ticks = 1);
@@ -267,7 +252,6 @@ abstract class PhabricatorAphlictManagementWorkflow
     fclose(STDOUT);
     fclose(STDERR);
 
-
     $this->launch();
     return 0;
   }
@@ -325,4 +309,17 @@ abstract class PhabricatorAphlictManagementWorkflow
         '$PATH'));
   }
 
+  private function getAphlictScriptPath() {
+    $root = dirname(phutil_get_library_root('phabricator'));
+    return $root.'/support/aphlict/server/aphlict_server.js';
+  }
+
+  private function getStartCommand(array $server_argv) {
+    return csprintf(
+      '%s %s %Ls',
+      $this->getNodeBinary(),
+      $this->getAphlictScriptPath(),
+      $server_argv);
+  }
+
 }