1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Fix project hashtag regexp to stop matching terminal periods

Summary:
Fixes T6416. The comment is consistent with intent, but the actual regexp doesn't quite work right. In particular, we incorrectly match `#security.` as `security.` (with a period) instead of `security` (with no period).

Since this stuff is a pain to test and I evidently got it wrong in this case in D8703, make it unit testable.

Test Plan:
Added unit tests. Also:

{F227181}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6416

Differential Revision: https://secure.phabricator.com/D10753
This commit is contained in:
epriestley 2014-10-29 08:13:38 -07:00
parent d5b70e2c1c
commit dc6b988dea
4 changed files with 131 additions and 11 deletions

View file

@ -2823,6 +2823,7 @@ phutil_register_library_map(array(
'ProjectCreateProjectsCapability' => 'applications/project/capability/ProjectCreateProjectsCapability.php',
'ProjectQueryConduitAPIMethod' => 'applications/project/conduit/ProjectQueryConduitAPIMethod.php',
'ProjectRemarkupRule' => 'applications/project/remarkup/ProjectRemarkupRule.php',
'ProjectRemarkupRuleTestCase' => 'applications/project/remarkup/__tests__/ProjectRemarkupRuleTestCase.php',
'QueryFormattingTestCase' => 'infrastructure/storage/__tests__/QueryFormattingTestCase.php',
'ReleephAuthorFieldSpecification' => 'applications/releeph/field/specification/ReleephAuthorFieldSpecification.php',
'ReleephBranch' => 'applications/releeph/storage/ReleephBranch.php',
@ -6015,6 +6016,7 @@ phutil_register_library_map(array(
'ProjectCreateProjectsCapability' => 'PhabricatorPolicyCapability',
'ProjectQueryConduitAPIMethod' => 'ProjectConduitAPIMethod',
'ProjectRemarkupRule' => 'PhabricatorObjectRemarkupRule',
'ProjectRemarkupRuleTestCase' => 'PhabricatorTestCase',
'QueryFormattingTestCase' => 'PhabricatorTestCase',
'ReleephAuthorFieldSpecification' => 'ReleephFieldSpecification',
'ReleephBranch' => array(

View file

@ -30,7 +30,7 @@ final class ProjectRemarkupRule extends PhabricatorObjectRemarkupRule {
// In other contexts, the PhabricatorProjectProjectPHIDType pattern is
// controlling and these names should parse correctly.
return '[^\s.!,:;{}#]*[^\s\d!,:;{}#]+(?:[^\s.!,:;{}#][^\s!,:;{}#]*)*';
return '[^\s.\d!,:;{}#]+(?:[^\s!,:;{}#][^\s.!,:;{}#]+)*';
}
protected function loadObjects(array $ids) {

View file

@ -0,0 +1,57 @@
<?php
final class ProjectRemarkupRuleTestCase extends PhabricatorTestCase {
public function testProjectObjectRemarkup() {
$cases = array(
'I like #ducks.' => array(
'embed' => array(),
'ref' => array(
array(
'offset' => 8,
'id' => 'ducks',
),
),
),
'We should make a post on #blog.example.com tomorrow.' => array(
'embed' => array(),
'ref' => array(
array(
'offset' => 26,
'id' => 'blog.example.com',
),
),
),
'We should make a post on #blog.example.com.' => array(
'embed' => array(),
'ref' => array(
array(
'offset' => 26,
'id' => 'blog.example.com',
),
),
),
'#123' => array(
'embed' => array(),
'ref' => array(),
),
'#security#123' => array(
'embed' => array(),
'ref' => array(
array(
'offset' => 1,
'id' => 'security',
'tail' => '123',
),
),
),
);
foreach ($cases as $input => $expect) {
$rule = new ProjectRemarkupRule();
$matches = $rule->extractReferences($input);
$this->assertEqual($expect, $matches, $input);
}
}
}

View file

@ -95,15 +95,33 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
}
public function apply($text) {
$prefix = $this->getObjectNamePrefix();
$prefix = preg_quote($prefix, '@');
$id = $this->getObjectIDPattern();
$text = preg_replace_callback(
'@\B{'.$prefix.'('.$id.')((?:[^}\\\\]|\\\\.)*)}\B@u',
$this->getObjectEmbedPattern(),
array($this, 'markupObjectEmbed'),
$text);
$text = preg_replace_callback(
$this->getObjectReferencePattern(),
array($this, 'markupObjectReference'),
$text);
return $text;
}
private function getObjectEmbedPattern() {
$prefix = $this->getObjectNamePrefix();
$prefix = preg_quote($prefix);
$id = $this->getObjectIDPattern();
return '(\B{'.$prefix.'('.$id.')((?:[^}\\\\]|\\\\.)*)}\B)u';
}
private function getObjectReferencePattern() {
$prefix = $this->getObjectNamePrefix();
$prefix = preg_quote($prefix);
$id = $this->getObjectIDPattern();
// If the prefix starts with a word character (like "D"), we want to
// require a word boundary so that we don't match "XD1" as "D1". If the
// prefix does not start with a word character, we want to require no word
@ -121,12 +139,55 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
// The "\b" allows us to link "(abcdef)" or similar without linking things
// in the middle of words.
$text = preg_replace_callback(
'((?<![#-])'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?(?!\w))u',
array($this, 'markupObjectReference'),
$text);
return '((?<![#-])'.$boundary.$prefix.'('.$id.')(?:#([-\w\d]+))?(?!\w))u';
}
return $text;
/**
* Extract matched object references from a block of text.
*
* This is intended to make it easy to write unit tests for object remarkup
* rules. Production code is not normally expected to call this method.
*
* @param string Text to match rules against.
* @return wild Matches, suitable for writing unit tests against.
*/
public function extractReferences($text) {
$embed_matches = null;
preg_match_all(
$this->getObjectEmbedPattern(),
$text,
$embed_matches,
PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
$ref_matches = null;
preg_match_all(
$this->getObjectReferencePattern(),
$text,
$ref_matches,
PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
$results = array();
$sets = array(
'embed' => $embed_matches,
'ref' => $ref_matches,
);
foreach ($sets as $type => $matches) {
$formatted = array();
foreach ($matches as $match) {
$format = array(
'offset' => $match[1][1],
'id' => $match[1][0],
);
if (isset($match[2][0])) {
$format['tail'] = $match[2][0];
}
$formatted[] = $format;
}
$results[$type] = $formatted;
}
return $results;
}
public function markupObjectEmbed($matches) {