From 06c641f92c65bb833080f84083a58e3ce7b227ae Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 Jul 2016 05:39:39 -0700 Subject: [PATCH] Remove command spelling correction from Arcanist Summary: Fixes T7489. Depends on D16332, which moved this code to libphutil. Test Plan: ``` $ arc banch --bystatus (Assuming 'banch' is the British spelling of 'branch'.) (Assuming '--bystatus' is the British spelling of '--by-status'.) ... ``` Reviewers: chad Reviewed By: chad Maniphest Tasks: T7489 Differential Revision: https://secure.phabricator.com/D16333 --- src/__phutil_library_map__.php | 2 - src/configuration/ArcanistConfiguration.php | 77 +-------------- .../__tests__/ArcanistBritishTestCase.php | 97 ------------------- src/workflow/ArcanistWorkflow.php | 5 +- 4 files changed, 4 insertions(+), 177 deletions(-) delete mode 100644 src/configuration/__tests__/ArcanistBritishTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cd419223..d8f10dd4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -41,7 +41,6 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistBraceFormattingXHPASTLinterRule.php', 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php', 'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php', - 'ArcanistBritishTestCase' => 'configuration/__tests__/ArcanistBritishTestCase.php', 'ArcanistBrowseWorkflow' => 'workflow/ArcanistBrowseWorkflow.php', 'ArcanistBundle' => 'parser/ArcanistBundle.php', 'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php', @@ -456,7 +455,6 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow', - 'ArcanistBritishTestCase' => 'PhutilTestCase', 'ArcanistBrowseWorkflow' => 'ArcanistWorkflow', 'ArcanistBundle' => 'Phobject', 'ArcanistBundleTestCase' => 'PhutilTestCase', diff --git a/src/configuration/ArcanistConfiguration.php b/src/configuration/ArcanistConfiguration.php index 8761d8cb..80ebf6fa 100644 --- a/src/configuration/ArcanistConfiguration.php +++ b/src/configuration/ArcanistConfiguration.php @@ -144,7 +144,8 @@ class ArcanistConfiguration extends Phobject { // We haven't found a real command, alias, or unique prefix. Try similar // spellings. - $corrected = self::correctCommandSpelling($command, $all, 2); + $corrected = PhutilArgumentSpellingCorrector::newCommandCorrector() + ->correctSpelling($command, $all); if (count($corrected) == 1) { $console->writeErr( pht( @@ -183,78 +184,4 @@ class ArcanistConfiguration extends Phobject { return array_keys($is_prefix); } - public static function correctCommandSpelling( - $command, - array $options, - $max_distance) { - - // Adjust to the scaled edit costs we use below, so "2" roughly means - // "2 edits". - $max_distance = $max_distance * 3; - - // These costs are somewhat made up, but the theory is that it is far more - // likely you will mis-strike a key ("lans" for "land") or press two keys - // out of order ("alnd" for "land") than omit keys or press extra keys. - $matrix = id(new PhutilEditDistanceMatrix()) - ->setInsertCost(4) - ->setDeleteCost(4) - ->setReplaceCost(3) - ->setTransposeCost(2); - - return self::correctSpelling($command, $options, $matrix, $max_distance); - } - - public static function correctArgumentSpelling($command, array $options) { - $max_distance = 1; - - // We are stricter with arguments - we allow only one inserted or deleted - // character. It is mainly to handle cases like --no-lint versus --nolint - // or --reviewer versus --reviewers. - $matrix = id(new PhutilEditDistanceMatrix()) - ->setInsertCost(1) - ->setDeleteCost(1) - ->setReplaceCost(10); - - return self::correctSpelling($command, $options, $matrix, $max_distance); - } - - public static function correctSpelling( - $input, - array $options, - PhutilEditDistanceMatrix $matrix, - $max_distance) { - - $distances = array(); - $inputv = str_split($input); - foreach ($options as $option) { - $optionv = str_split($option); - $matrix->setSequences($optionv, $inputv); - $distances[$option] = $matrix->getEditDistance(); - } - - asort($distances); - $best = min($max_distance, reset($distances)); - foreach ($distances as $option => $distance) { - if ($distance > $best) { - unset($distances[$option]); - } - } - - // Before filtering, check if we have multiple equidistant matches and - // return them if we do. This prevents us from, e.g., matching "alnd" with - // both "land" and "amend", then dropping "land" for being too short, and - // incorrectly completing to "amend". - if (count($distances) > 1) { - return array_keys($distances); - } - - foreach ($distances as $option => $distance) { - if (strlen($option) < $distance) { - unset($distances[$option]); - } - } - - return array_keys($distances); - } - } diff --git a/src/configuration/__tests__/ArcanistBritishTestCase.php b/src/configuration/__tests__/ArcanistBritishTestCase.php deleted file mode 100644 index 7cba7407..00000000 --- a/src/configuration/__tests__/ArcanistBritishTestCase.php +++ /dev/null @@ -1,97 +0,0 @@ -assertCommandCompletion( - array('land'), - 'alnd', - array('land', 'amend')); - - $this->assertCommandCompletion( - array('branch'), - 'brnach', - array('branch', 'browse')); - - $this->assertCommandCompletion( - array(), - 'test', - array('list', 'unit')); - - $this->assertCommandCompletion( - array('list'), - 'lists', - array('list')); - - $this->assertCommandCompletion( - array('diff'), - 'dfif', - array('diff')); - - $this->assertCommandCompletion( - array('unit'), - 'uint', - array('unit', 'lint', 'list')); - - $this->assertCommandCompletion( - array('list', 'lint'), - 'nilt', - array('unit', 'lint', 'list')); - } - - private function assertCommandCompletion($expect, $input, $commands) { - $result = ArcanistConfiguration::correctCommandSpelling( - $input, - $commands, - 2); - - sort($result); - sort($expect); - - $commands = implode(', ', $commands); - - $this->assertEqual( - $expect, - $result, - pht('Correction of %s against: %s', $input, $commands)); - } - - public function testArgumentCompletion() { - $this->assertArgumentCompletion( - array('nolint'), - 'no-lint', - array('nolint', 'nounit')); - - $this->assertArgumentCompletion( - array('reviewers'), - 'reviewer', - array('reviewers', 'cc')); - - $this->assertArgumentCompletion( - array(), - 'onlint', - array('nolint')); - - $this->assertArgumentCompletion( - array(), - 'nolind', - array('nolint')); - } - - private function assertArgumentCompletion($expect, $input, $arguments) { - $result = ArcanistConfiguration::correctArgumentSpelling( - $input, - $arguments); - - sort($result); - sort($expect); - - $arguments = implode(', ', $arguments); - - $this->assertEqual( - $expect, - $result, - pht('Correction of %s against: %s', $input, $arguments)); - } - -} diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 366f4742..b4b105d6 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -673,9 +673,8 @@ abstract class ArcanistWorkflow extends Phobject { } if (!array_key_exists($arg_key, $spec)) { - $corrected = ArcanistConfiguration::correctArgumentSpelling( - $arg_key, - array_keys($spec)); + $corrected = PhutilArgumentSpellingCorrector::newFlagCorrector() + ->correctSpelling($arg_key, array_keys($spec)); if (count($corrected) == 1) { PhutilConsole::getConsole()->writeErr( pht(