Skip to content

Commit

Permalink
Quite a bit of refactoring as unit tests get added
Browse files Browse the repository at this point in the history
  • Loading branch information
stuartherbert committed Apr 19, 2011
1 parent 5559373 commit c277d43
Show file tree
Hide file tree
Showing 21 changed files with 55 additions and 306 deletions.
2 changes: 1 addition & 1 deletion build.xml
Expand Up @@ -130,7 +130,7 @@
<delete dir="${project.review.codecoveragedir}" /> <delete dir="${project.review.codecoveragedir}" />
<mkdir dir="${project.review.codecoveragedir}" /> <mkdir dir="${project.review.codecoveragedir}" />
<mkdir dir="${project.review.logsdir}" /> <mkdir dir="${project.review.logsdir}" />
<exec command="phpunit --bootstrap=${project.src.testunitdir}/bootstrap.php --coverage-html ${project.review.codecoveragedir} --coverage-clover ${project.review.logsdir}/phpunit.xml ${project.src.testunitdir}" checkreturn="true" logoutput="true"/> <exec command="phpunit --bootstrap=${project.src.testunitdir}/bootstrap.php --coverage-html ${project.review.codecoveragedir} --coverage-clover ${project.review.logsdir}/phpunit.xml --testdox ${project.src.testunitdir}" checkreturn="true" logoutput="true"/>
<echo/> <echo/>
<echo>The code coverage report is in ${project.review.codecoveragedir}</echo> <echo>The code coverage report is in ${project.review.codecoveragedir}</echo>
</target> </target>
Expand Down
Expand Up @@ -44,7 +44,7 @@


namespace Phix_Project\Phix; namespace Phix_Project\Phix;


class ExtensionsFinder class CommandsFinder
{ {
protected $foldersToSearch = array(); protected $foldersToSearch = array();


Expand All @@ -66,7 +66,7 @@ public function addFolderToSearch($path)
$this->foldersToSearch[$path] = $path; $this->foldersToSearch[$path] = $path;
} }


public function findExtensions() public function findCommands()
{ {
$commandsList = new CommandsList(); $commandsList = new CommandsList();


Expand All @@ -77,17 +77,17 @@ public function findExtensions()
{ {
include_once $filename; include_once $filename;


if ($this->testIsPhixExtension($newClass)) if ($this->testIsPhixCommand($newClass))
{ {
// we have a winner! // we have a winner!
$commandsList->importCommandsFromExtension($newClass); $commandsList->addClass($newClass);
} }
} }


return $commandsList; return $commandsList;
} }


protected function testIsPhixExtension($className) protected function testIsPhixCommand($className)
{ {
$refObj = new \ReflectionClass($className); $refObj = new \ReflectionClass($className);
if ($refObj->implementsInterface('\Phix_Project\PhixExtensions\CommandInterface')) if ($refObj->implementsInterface('\Phix_Project\PhixExtensions\CommandInterface'))
Expand All @@ -97,4 +97,4 @@ protected function testIsPhixExtension($className)


return false; return false;
} }
} }
6 changes: 3 additions & 3 deletions src/php/Phix_Project/Phix/CommandsList.php
Expand Up @@ -54,10 +54,10 @@ class CommandsList
{ {
public $commands = array(); public $commands = array();


public function importCommandsFromExtension($PhixExtensionClassName) public function addClass($phixCommandClassName)
{ {
// what is this command all about? // what is this command all about?
$newCommand = new $PhixExtensionClassName; $newCommand = new $phixCommandClassName;
$commandName = $newCommand->getCommandName(); $commandName = $newCommand->getCommandName();
$commandDesc = $newCommand->getCommandDesc(); $commandDesc = $newCommand->getCommandDesc();


Expand Down Expand Up @@ -88,4 +88,4 @@ public function getListOfCommands()
{ {
return $this->commands; return $this->commands;
} }
} }
43 changes: 21 additions & 22 deletions src/php/Phix_Project/Phix/Phix.php
Expand Up @@ -73,29 +73,36 @@ public function main($argv)
// step 1: parse the command-line args // step 1: parse the command-line args
// we do this first because it may change where we look // we do this first because it may change where we look
// for our commands // for our commands
$context->phixDefinedSwitches = $this->buildPhixSwitches(); $context->phixDefinedSwitches = PhixSwitches::buildSwitches();
list($phixParsedSwitches, $argsIndex) = $this->parsePhixArgs($context, $argv); list($phixParsedSwitches, $argsIndex) = $this->parsePhixArgs($context, $argv);


// step 2: process the switches we have just parsed // step 2: process the switches we have just parsed
// //
// we parse the switches twice; once to find out where to // we parse the switches twice; once to find out where to
// look for extensions, and then once more to decide what // look for extensions, and then once more to decide what
// to do once the extensions are loaded // to do once the extensions are loaded
$errCode = $this->processPhixSwitchesBeforeExtensionLoad($context, $phixParsedSwitches, $argv, $argsIndex); $errCode = $this->processPhixSwitchesBeforeCommandLoad($context, $phixParsedSwitches, $argv, $argsIndex);
if ($errCode !== null) if ($errCode !== null)
{ {
return $errCode; return $errCode;
} }


$context->commandsList = $this->loadPhixExtensions($context, $phixParsedSwitches); // step 3: load the phix commands
//
// we do this here in case anyone has used the -I switch
$context->commandsList = $this->loadPhixCommands($context, $phixParsedSwitches);


$errCode = $this->processPhixSwitchesAfterExtensionLoad($context, $phixParsedSwitches); // step 4: process the switches a second time
//
// just in case anything ever wants to happen after we have
// loaded a list of available commands
$errCode = $this->processPhixSwitchesAfterCommandLoad($context, $phixParsedSwitches);
if ($errCode !== null) if ($errCode !== null)
{ {
return $errCode; return $errCode;
} }


// step 3: do we have a valid command to execute? // step 5: do we have a valid command to execute?
if (!isset($argv[$argsIndex])) if (!isset($argv[$argsIndex]))
{ {
// no command given - special case // no command given - special case
Expand All @@ -107,21 +114,13 @@ public function main($argv)
return $errCode; return $errCode;
} }


// step 4: execute the validated command // step 6: execute the validated command
$errCode = $this->processCommand($argv, $argsIndex, $context); $errCode = $this->processCommand($argv, $argsIndex, $context);


// all done // all done
return $errCode; return $errCode;
} }


protected function buildPhixSwitches()
{
$switches = PhixSwitches::buildSwitches();

// all done
return $switches;
}

protected function parsePhixArgs(Context $context, $argv) protected function parsePhixArgs(Context $context, $argv)
{ {
// switches before the first command are switches that // switches before the first command are switches that
Expand All @@ -134,7 +133,7 @@ protected function parsePhixArgs(Context $context, $argv)
return $parser->parseSwitches($argv, 1, $context->phixDefinedSwitches); return $parser->parseSwitches($argv, 1, $context->phixDefinedSwitches);
} }


protected function processPhixSwitchesBeforeExtensionLoad(Context $context, ParsedSwitches $ParsedSwitches, $argv, $argsIndex) protected function processPhixSwitchesBeforeCommandLoad(Context $context, ParsedSwitches $ParsedSwitches, $argv, $argsIndex)
{ {
// what switches do we have? // what switches do we have?
$parsedSwitches = $ParsedSwitches->getSwitches(); $parsedSwitches = $ParsedSwitches->getSwitches();
Expand All @@ -144,7 +143,7 @@ protected function processPhixSwitchesBeforeExtensionLoad(Context $context, Pars
{ {
$switchName = $parsedSwitch->name; $switchName = $parsedSwitch->name;
$className = '\\Phix_Project\\PhixSwitches\\' . ucfirst($switchName) . 'Switch'; $className = '\\Phix_Project\\PhixSwitches\\' . ucfirst($switchName) . 'Switch';
$errCode = $className::processBeforeExtensionLoad($context, $ParsedSwitches->getArgsForSwitch($switchName), $argv, $argsIndex); $errCode = $className::processBeforeCommandLoad($context, $ParsedSwitches->getArgsForSwitch($switchName), $argv, $argsIndex);
if ($errCode !== null) if ($errCode !== null)
{ {
return $errCode; return $errCode;
Expand All @@ -156,10 +155,10 @@ protected function processPhixSwitchesBeforeExtensionLoad(Context $context, Pars
return null; return null;
} }


protected function loadPhixExtensions(Context $context, ParsedSwitches $ParsedSwitches) protected function loadPhixCommands(Context $context, ParsedSwitches $ParsedSwitches)
{ {
// create something to find the commands // create something to find the commands
$extensionsFinder = new ExtensionsFinder(); $commandsFinder = new CommandsFinder();


// seed the commandsFinder with a list of where to look // seed the commandsFinder with a list of where to look
// if the user has given us any hints // if the user has given us any hints
Expand All @@ -170,19 +169,19 @@ protected function loadPhixExtensions(Context $context, ParsedSwitches $ParsedSw


foreach ($args as $path) foreach ($args as $path)
{ {
$extensionsFinder->addFolderToSearch($path); $commandsFinder->addFolderToSearch($path);
} }
} }


// alright, let's thrash that hard disk // alright, let's thrash that hard disk
// hope you have an SSD ;-) // hope you have an SSD ;-)
$commandsList = $extensionsFinder->findExtensions(); $commandsList = $commandsFinder->findCommands();


// all done // all done
return $commandsList; return $commandsList;
} }


protected function processPhixSwitchesAfterExtensionLoad(Context $context, ParsedSwitches $ParsedSwitches) protected function processPhixSwitchesAfterCommandLoad(Context $context, ParsedSwitches $ParsedSwitches)
{ {
// what switches do we have? // what switches do we have?
$parsedSwitches = $ParsedSwitches->getSwitches(); $parsedSwitches = $ParsedSwitches->getSwitches();
Expand All @@ -192,7 +191,7 @@ protected function processPhixSwitchesAfterExtensionLoad(Context $context, Parse
{ {
$switchName = $parsedSwitch->name; $switchName = $parsedSwitch->name;
$className = '\\Phix_Project\\PhixSwitches\\' . ucfirst($switchName) . 'Switch'; $className = '\\Phix_Project\\PhixSwitches\\' . ucfirst($switchName) . 'Switch';
$errCode = $className::processAfterExtensionLoad($context, $ParsedSwitches->getArgsForSwitch($switchName)); $errCode = $className::processAfterCommandLoad($context, $ParsedSwitches->getArgsForSwitch($switchName));
if ($errCode !== null) if ($errCode !== null)
{ {
return $errCode; return $errCode;
Expand Down
70 changes: 2 additions & 68 deletions src/php/Phix_Project/PhixCommands/HelpCommand.php
Expand Up @@ -98,14 +98,13 @@ public function validateAndExecute($args, $argsIndex, Context $context)
$command = $context->commandsList->getCommand($commandForHelp); $command = $context->commandsList->getCommand($commandForHelp);
$command->outputHelp($context); $command->outputHelp($context);



return 0; return 0;
} }


protected function showGeneralHelp(Context $context) protected function showGeneralHelp(Context $context)
{ {
// get the list of switches in display order // get the list of switches in display order
$sortedSwitches = $this->calculatePhixSwitchDisplayOrder($context); $sortedSwitches = $context->phixDefinedSwitches->getSwitchesInDisplayOrder();


$so = $context->stdout; $so = $context->stdout;


Expand All @@ -118,71 +117,6 @@ protected function showGeneralHelp(Context $context)
$this->showCommandsList($context); $this->showCommandsList($context);
} }


protected function calculatePhixSwitchDisplayOrder(Context $context)
{
// turn the list into something that's suitably sorted
$shortSwitchesWithoutArgs = array();
$shortSwitchesWithArgs = array();
$longSwitchesWithoutArgs = array();
$longSwitchesWithArgs = array();

$allShortSwitches = array();
$allLongSwitches = array();

$allSwitches = $context->phixDefinedSwitches->getSwitches();

foreach ($allSwitches as $switch)
{
foreach ($switch->shortSwitches as $shortSwitch)
{
$allShortSwitches['-' . $shortSwitch] = $switch;

if ($switch->testHasArgument())
{
$shortSwitchesWithArgs[$shortSwitch] = $switch;
}
else
{
$shortSwitchesWithoutArgs[$shortSwitch] = $shortSwitch;
}
}

foreach ($switch->longSwitches as $longSwitch)
{
$allLongSwitches['--' . $longSwitch] = $switch;

if ($switch->testHasArgument())
{
$longSwitchesWithArgs[$longSwitch] = $switch;
}
else
{
$longSwitchesWithoutArgs[$longSwitch] = $longSwitch;
}
}
}

// we have all the switches that phix supports
// let's put them into sensible orders, and then display
// them
\ksort($shortSwitchesWithArgs);
\ksort($shortSwitchesWithoutArgs);
\ksort($longSwitchesWithArgs);
\ksort($longSwitchesWithoutArgs);
\ksort($allShortSwitches);
\ksort($allLongSwitches);

$return = array (
'shortSwitchesWithArgs' => $shortSwitchesWithArgs,
'shortSwitchesWithoutArgs' => $shortSwitchesWithoutArgs,
'longSwitchesWithArgs' => $longSwitchesWithArgs,
'longSwitchesWithoutArgs' => $longSwitchesWithoutArgs,
'allSwitches' => array_merge($allShortSwitches, $allLongSwitches),
);

return $return;
}

protected function showPhixSwitchSummary(Context $context, $sortedSwitches) protected function showPhixSwitchSummary(Context $context, $sortedSwitches)
{ {
$so = $context->stdout; $so = $context->stdout;
Expand Down Expand Up @@ -343,7 +277,7 @@ protected function showCommandsList(Context $context)
$so->outputLine(null, ' for detailed help on <command>'); $so->outputLine(null, ' for detailed help on <command>');
} }


protected function outputHelp(Context $context) public function outputHelp(Context $context)
{ {
$so = $context->stdout; $so = $context->stdout;


Expand Down
67 changes: 1 addition & 66 deletions src/php/Phix_Project/PhixExtensions/CommandBase.php
Expand Up @@ -96,7 +96,7 @@ public function outputHelp(Context $context)
$sortedSwitches = null; $sortedSwitches = null;
if ($options !== null) if ($options !== null)
{ {
$sortedSwitches = $this->calculateSwitchDisplayOrder($options); $sortedSwitches = $options->getSwitchesInDisplayOrder();
} }


$this->showName($context); $this->showName($context);
Expand All @@ -105,71 +105,6 @@ public function outputHelp(Context $context)
$this->showImplementationDetails($context); $this->showImplementationDetails($context);
} }


protected function calculateSwitchDisplayOrder(DefinedSwitches $options)
{
// turn the list into something that's suitably sorted
$shortSwitchesWithoutArgs = array();
$shortSwitchesWithArgs = array();
$longSwitchesWithoutArgs = array();
$longSwitchesWithArgs = array();

$allShortSwitches = array();
$allLongSwitches = array();

$allSwitches = $options->getSwitches();

foreach ($allSwitches as $switch)
{
foreach ($switch->shortSwitches as $shortSwitch)
{
$allShortSwitches['-' . $shortSwitch] = $switch;

if ($switch->testHasArgument())
{
$shortSwitchesWithArgs[$shortSwitch] = $switch;
}
else
{
$shortSwitchesWithoutArgs[$shortSwitch] = $shortSwitch;
}
}

foreach ($switch->longSwitches as $longSwitch)
{
$allLongSwitches['--' . $longSwitch] = $switch;

if ($switch->testHasArgument())
{
$longSwitchesWithArgs[$longSwitch] = $switch;
}
else
{
$longSwitchesWithoutArgs[$longSwitch] = $longSwitch;
}
}
}

// we have all the switches that phix supports
// let's put them into sensible orders, and then display
// them
\ksort($shortSwitchesWithArgs);
\ksort($shortSwitchesWithoutArgs);
\ksort($longSwitchesWithArgs);
\ksort($longSwitchesWithoutArgs);
\ksort($allShortSwitches);
\ksort($allLongSwitches);

$return = array (
'shortSwitchesWithArgs' => $shortSwitchesWithArgs,
'shortSwitchesWithoutArgs' => $shortSwitchesWithoutArgs,
'longSwitchesWithArgs' => $longSwitchesWithArgs,
'longSwitchesWithoutArgs' => $longSwitchesWithoutArgs,
'allSwitches' => array_merge($allShortSwitches, $allLongSwitches),
);

return $return;
}

protected function showName(Context $context) protected function showName(Context $context)
{ {
$so = $context->stdout; $so = $context->stdout;
Expand Down

0 comments on commit c277d43

Please sign in to comment.