diff --git a/resources/sql/autopatches/20141107.phriction.policy.2.php b/resources/sql/autopatches/20141107.phriction.policy.2.php
--- a/resources/sql/autopatches/20141107.phriction.policy.2.php
+++ b/resources/sql/autopatches/20141107.phriction.policy.2.php
@@ -24,12 +24,14 @@
$prefix = 'projects/';
if (($slug != $prefix) && (strncmp($slug, $prefix, strlen($prefix)) === 0)) {
$parts = explode('/', $slug);
- $project_slug = $parts[1].'/';
+
+ $project_slug = $parts[1];
+ $project_slug = PhabricatorSlug::normalizeProjectSlug($project_slug);
$project_slugs = array($project_slug);
$project = id(new PhabricatorProjectQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPhrictionSlugs($project_slugs)
+ ->withSlugs($project_slugs)
->executeOne();
if ($project) {
diff --git a/resources/sql/patches/090.forceuniqueprojectnames.php b/resources/sql/patches/090.forceuniqueprojectnames.php
--- a/resources/sql/patches/090.forceuniqueprojectnames.php
+++ b/resources/sql/patches/090.forceuniqueprojectnames.php
@@ -10,13 +10,14 @@
$slug_map = array();
foreach ($projects as $project) {
- $project->setPhrictionSlug($project->getName());
- $slug = $project->getPhrictionSlug();
- if ($slug == '/') {
+ $slug = PhabricatorSlug::normalizeProjectSlug($project->getName());
+
+ if (!strlen($slug)) {
$project_id = $project->getID();
echo pht("Project #%d doesn't have a meaningful name...", $project_id)."\n";
$project->setName(trim(pht('Unnamed Project %s', $project->getName())));
}
+
$slug_map[$slug][] = $project->getID();
}
@@ -47,8 +48,8 @@
foreach ($update as $key => $project) {
$id = $project->getID();
$name = $project->getName();
- $project->setPhrictionSlug($name);
- $slug = $project->getPhrictionSlug();
+
+ $slug = PhabricatorSlug::normalizeProjectSlug($name).'/';
echo pht("Updating project #%d '%s' (%s)... ", $id, $name, $slug);
try {
@@ -87,8 +88,8 @@
$suffix = 2;
while (true) {
$new_name = $project->getName().' ('.$suffix.')';
- $project->setPhrictionSlug($new_name);
- $new_slug = $project->getPhrictionSlug();
+
+ $new_slug = PhabricatorSlug::normalizeProjectSlug($new_name).'/';
$okay = true;
foreach ($projects as $other) {
diff --git a/src/applications/project/conduit/ProjectQueryConduitAPIMethod.php b/src/applications/project/conduit/ProjectQueryConduitAPIMethod.php
--- a/src/applications/project/conduit/ProjectQueryConduitAPIMethod.php
+++ b/src/applications/project/conduit/ProjectQueryConduitAPIMethod.php
@@ -112,7 +112,7 @@
$slug_map = array();
if ($slugs) {
foreach ($slugs as $slug) {
- $normal = rtrim(PhabricatorSlug::normalize($slug), '/');
+ $normal = PhabricatorSlug::normalizeProjectSlug($slug);
foreach ($projects as $project) {
if (in_array($normal, $project['slugs'])) {
$slug_map[$slug] = $project['phid'];
diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
--- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
+++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php
@@ -81,9 +81,9 @@
switch ($xaction->getTransactionType()) {
case PhabricatorProjectTransaction::TYPE_NAME:
- $object->setName($xaction->getNewValue());
- // TODO - this is really "setPrimarySlug"
- $object->setPhrictionSlug($xaction->getNewValue());
+ $name = $xaction->getNewValue();
+ $object->setName($name);
+ $object->setPrimarySlug(PhabricatorSlug::normalizeProjectSlug($name));
return;
case PhabricatorProjectTransaction::TYPE_SLUGS:
return;
@@ -265,9 +265,8 @@
$errors[] = $error;
}
- $slug_builder = clone $object;
- $slug_builder->setPhrictionSlug($name);
- $slug = $slug_builder->getPrimarySlug();
+ $slug = PhabricatorSlug::normalizeProjectSlug($name);
+
$slug_used_already = id(new PhabricatorProjectSlug())
->loadOneWhere('slug = %s', $slug);
if ($slug_used_already &&
diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php
--- a/src/applications/project/query/PhabricatorProjectQuery.php
+++ b/src/applications/project/query/PhabricatorProjectQuery.php
@@ -7,7 +7,6 @@
private $phids;
private $memberPHIDs;
private $slugs;
- private $phrictionSlugs;
private $names;
private $nameTokens;
private $icons;
@@ -50,11 +49,6 @@
return $this;
}
- public function withPhrictionSlugs(array $slugs) {
- $this->phrictionSlugs = $slugs;
- return $this;
- }
-
public function withNames(array $names) {
$this->names = $names;
return $this;
@@ -308,13 +302,6 @@
$this->slugs);
}
- if ($this->phrictionSlugs !== null) {
- $where[] = qsprintf(
- $conn,
- 'phrictionSlug IN (%Ls)',
- $this->phrictionSlugs);
- }
-
if ($this->names !== null) {
$where[] = qsprintf(
$conn,
diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php
--- a/src/applications/project/storage/PhabricatorProject.php
+++ b/src/applications/project/storage/PhabricatorProject.php
@@ -189,24 +189,11 @@
return $this->assertAttached($this->memberPHIDs);
}
- public function setPhrictionSlug($slug) {
-
- // NOTE: We're doing a little magic here and stripping out '/' so that
- // project pages always appear at top level under projects/ even if the
- // display name is "Hack / Slash" or similar (it will become
- // 'hack_slash' instead of 'hack/slash').
-
- $slug = str_replace('/', ' ', $slug);
- $slug = PhabricatorSlug::normalize($slug);
- $this->phrictionSlug = $slug;
+ public function setPrimarySlug($slug) {
+ $this->phrictionSlug = $slug.'/';
return $this;
}
- public function getFullPhrictionSlug() {
- $slug = $this->getPhrictionSlug();
- return 'projects/'.$slug;
- }
-
// TODO - once we sever project => phriction automagicalness,
// migrate getPhrictionSlug to have no trailing slash and be called
// getPrimarySlug
diff --git a/src/infrastructure/util/PhabricatorSlug.php b/src/infrastructure/util/PhabricatorSlug.php
--- a/src/infrastructure/util/PhabricatorSlug.php
+++ b/src/infrastructure/util/PhabricatorSlug.php
@@ -4,21 +4,54 @@
public static function normalizeProjectSlug($slug) {
$slug = str_replace('/', ' ', $slug);
- $slug = self::normalize($slug);
+ $slug = self::normalize($slug, $hashtag = true);
return rtrim($slug, '/');
}
- public static function normalize($slug) {
+ public static function normalize($slug, $hashtag = false) {
$slug = preg_replace('@/+@', '/', $slug);
$slug = trim($slug, '/');
$slug = phutil_utf8_strtolower($slug);
- $slug = preg_replace("@[\\x00-\\x19#%&+=\\\\?<> ]+@", '_', $slug);
+
+ $ban =
+ // Ban control characters since users can't type them and they create
+ // various other problems with parsing and rendering.
+ "\\x00-\\x19".
+
+ // Ban characters with special meanings in URIs (and spaces), since we
+ // want slugs to produce nice URIs.
+ "#%&+=?".
+ " ".
+
+ // Ban backslashes and various brackets for parsing and URI quality.
+ "\\\\".
+ "<>{}\\[\\]".
+
+ // Ban single and double quotes since they can mess up URIs.
+ "'".
+ '"';
+
+ // In hashtag mode (used for Project hashtags), ban additional characters
+ // which cause parsing problems.
+ if ($hashtag) {
+ $ban .= '`~!@$^*,:;(|)';
+ }
+
+ $slug = preg_replace('(['.$ban.']+)', '_', $slug);
$slug = preg_replace('@_+@', '_', $slug);
+ $parts = explode('/', $slug);
+
+ // Convert "_-_" into "-". This is a little nicer for inputs with
+ // hyphens used as internal separators, and turns an input like "A - B"
+ // into "a-b" instead of "a_-_b";
+ foreach ($parts as $key => $part) {
+ $parts[$key] = str_replace('_-_', '-', $part);
+ }
+
// Remove leading and trailing underscores from each component, if the
// component has not been reduced to a single underscore. For example, "a?"
// converts to "a", but "??" converts to "_".
- $parts = explode('/', $slug);
foreach ($parts as $key => $part) {
if ($part != '_') {
$parts[$key] = trim($part, '_');
diff --git a/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php b/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
--- a/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
+++ b/src/infrastructure/util/__tests__/PhabricatorSlugTestCase.php
@@ -34,6 +34,9 @@
'a/??/c' => 'a/_/c/',
'a/?b/c' => 'a/b/c/',
'a/b?/c' => 'a/b/c/',
+ 'a - b' => 'a-b/',
+ 'a[b]' => 'a_b/',
+ 'ab!' => 'ab!/',
);
foreach ($slugs as $slug => $normal) {
@@ -44,6 +47,24 @@
}
}
+ public function testProjectSlugs() {
+ $slugs = array(
+ 'a:b' => 'a_b',
+ 'a!b' => 'a_b',
+ 'a - b' => 'a-b',
+ '' => '',
+ 'Demonology: HSA (Hexes, Signs, Alchemy)' =>
+ 'demonology_hsa_hexes_signs_alchemy',
+ );
+
+ foreach ($slugs as $slug => $normal) {
+ $this->assertEqual(
+ $normal,
+ PhabricatorSlug::normalizeProjectSlug($slug),
+ pht('Hashtag normalization of "%s"', $slug));
+ }
+ }
+
public function testSlugAncestry() {
$slugs = array(
'/' => array(),