diff --git a/src/applications/differential/customfield/DifferentialCustomField.php b/src/applications/differential/customfield/DifferentialCustomField.php index b4ef47c274..f80798d0e6 100644 --- a/src/applications/differential/customfield/DifferentialCustomField.php +++ b/src/applications/differential/customfield/DifferentialCustomField.php @@ -54,7 +54,7 @@ abstract class DifferentialCustomField if ($handle->getPolicyFiltered()) { $out[] = $handle->getPHID(); } else if ($handle->isComplete()) { - $out[] = $handle->getObjectName(); + $out[] = $handle->getCommandLineObjectName(); } } diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php index 9ab77c26ac..749f1e7ace 100644 --- a/src/applications/differential/customfield/DifferentialReviewersField.php +++ b/src/applications/differential/customfield/DifferentialReviewersField.php @@ -146,6 +146,7 @@ final class DifferentialReviewersField array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, + PhabricatorOwnersPackagePHIDType::TYPECONST, )); } diff --git a/src/applications/differential/customfield/DifferentialSubscribersField.php b/src/applications/differential/customfield/DifferentialSubscribersField.php index a1e8d1dbd6..b8423e9879 100644 --- a/src/applications/differential/customfield/DifferentialSubscribersField.php +++ b/src/applications/differential/customfield/DifferentialSubscribersField.php @@ -78,6 +78,7 @@ final class DifferentialSubscribersField array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, + PhabricatorOwnersPackagePHIDType::TYPECONST, )); } diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index aa5ae1c6f3..52f30e0994 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -205,6 +205,21 @@ final class PhabricatorOwnersPackageTransactionEditor $error->setIsMissingFieldError(true); $errors[] = $error; } + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + if (preg_match('([,!])', $new)) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Package names may not contain commas (",") or exclamation '. + 'marks ("!"). These characters are ambiguous when package '. + 'names are parsed from the command line.'), + $xaction); + } + } + break; case PhabricatorOwnersPackageTransaction::TYPE_PATHS: if (!$xactions) { diff --git a/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php b/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php index 891a9726c8..772bb84fc0 100644 --- a/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php +++ b/src/applications/owners/phid/PhabricatorOwnersPackagePHIDType.php @@ -40,9 +40,11 @@ final class PhabricatorOwnersPackagePHIDType extends PhabricatorPHIDType { $name = $package->getName(); $id = $package->getID(); - $handle->setName($monogram); - $handle->setFullName("{$monogram}: {$name}"); - $handle->setURI("/owners/package/{$id}/"); + $handle + ->setName($monogram) + ->setFullName("{$monogram}: {$name}") + ->setCommandLineObjectName("{$monogram} {$name}") + ->setURI("/owners/package/{$id}/"); if ($package->isArchived()) { $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 2b626f2e5d..f3c04d2a25 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -29,6 +29,7 @@ final class PhabricatorObjectHandle private $policyFiltered; private $subtitle; private $tokenIcon; + private $commandLineObjectName; public function setIcon($icon) { $this->icon = $icon; @@ -196,6 +197,19 @@ final class PhabricatorObjectHandle return $this->getName(); } + public function setCommandLineObjectName($command_line_object_name) { + $this->commandLineObjectName = $command_line_object_name; + return $this; + } + + public function getCommandLineObjectName() { + if ($this->commandLineObjectName !== null) { + return $this->commandLineObjectName; + } + + return $this->getObjectName(); + } + public function setTitle($title) { $this->title = $title; return $this; diff --git a/src/applications/phid/query/PhabricatorObjectListQuery.php b/src/applications/phid/query/PhabricatorObjectListQuery.php index c4bf0e9bba..a1fd3a82bd 100644 --- a/src/applications/phid/query/PhabricatorObjectListQuery.php +++ b/src/applications/phid/query/PhabricatorObjectListQuery.php @@ -45,9 +45,49 @@ final class PhabricatorObjectListQuery extends Phobject { public function execute() { $names = $this->getObjectList(); - $names = array_unique(array_filter(preg_split('/[\s,]+/', $names))); - $objects = $this->loadObjects($names); + // First, normalize any internal whitespace so we don't get weird results + // if linebreaks hit in weird spots. + $names = preg_replace('/\s+/', ' ', $names); + + // Split the list on commas. + $names = explode(',', $names); + + // Trim and remove empty tokens. + foreach ($names as $key => $name) { + $name = trim($name); + + if (!strlen($name)) { + unset($names[$key]); + continue; + } + + $names[$key] = $name; + } + + // Remove duplicates. + $names = array_unique($names); + + $name_map = array(); + foreach ($names as $name) { + $parts = explode(' ', $name); + + // If this looks like a monogram, ignore anything after the first token. + // This allows us to parse "O123 Package Name" as though it was "O123", + // which we can look up. + if (preg_match('/^[A-Z]\d+\z/', $parts[0])) { + $name_map[$parts[0]] = $name; + } else { + // For anything else, split it on spaces and use each token as a + // value. This means "alincoln htaft", separated with a space instead + // of with a comma, is two different users. + foreach ($parts as $part) { + $name_map[$part] = $part; + } + } + } + + $objects = $this->loadObjects(array_keys($name_map)); $types = array(); foreach ($objects as $name => $object) { @@ -66,8 +106,8 @@ final class PhabricatorObjectListQuery extends Phobject { $invalid = array_mergev($invalid); $missing = array(); - foreach ($names as $name) { - if (empty($objects[$name])) { + foreach ($name_map as $key => $name) { + if (empty($objects[$key])) { $missing[] = $name; } } diff --git a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php index e7347d5dbc..b47bedfa1e 100644 --- a/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php +++ b/src/applications/phid/query/__tests__/PhabricatorObjectListQueryTestCase.php @@ -13,7 +13,6 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { $name = $user->getUsername(); $phid = $user->getPHID(); - $result = $this->parseObjectList("@{$name}"); $this->assertEqual(array($phid), $result); @@ -29,6 +28,21 @@ final class PhabricatorObjectListQueryTestCase extends PhabricatorTestCase { $result = $this->parseObjectList(''); $this->assertEqual(array(), $result); + + $package = PhabricatorOwnersPackage::initializeNewPackage($user) + ->setName(pht('Query Test Package')) + ->save(); + + $package_phid = $package->getPHID(); + $package_mono = $package->getMonogram(); + + $result = $this->parseObjectList("{$package_mono} Any Ignored Text"); + $this->assertEqual(array($package_phid), $result); + + $result = $this->parseObjectList("{$package_mono} Any Text, {$name}"); + $this->assertEqual(array($package_phid, $phid), $result); + + // Expect failure when loading a user if objects must be of type "DUCK". $caught = null; try {