From 81d9935efe9268e3188cc239445549b3deccea14 Mon Sep 17 00:00:00 2001
From: Peng Li <pengli@fb.com>
Date: Thu, 6 Mar 2014 11:28:46 -0800
Subject: [PATCH] Allow parenthesis in author name

Summary: We have a dozen users who has `(...)` in their 'real name', like 'Jimmy (He) Zhang', and it's causing the diffusion file browser problems when blame is enabled. The parser does not expect those parenthesis and the lines of code will be empty if they were last touched by a user like that.

Test Plan: Try it

Reviewers: wez, lifeihuang, JoelB, #blessed_reviewers, epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8429
---
 src/__phutil_library_map__.php                |  2 ++
 .../DiffusionGitFileContentQueryTestCase.php  | 32 +++++++++++++++++++
 .../DiffusionGitFileContentQuery.php          |  6 +++-
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 6cc3dfa923..4ba16bbcaf 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -538,6 +538,7 @@ phutil_register_library_map(array(
     'DiffusionGitBranch' => 'applications/diffusion/data/DiffusionGitBranch.php',
     'DiffusionGitBranchTestCase' => 'applications/diffusion/data/__tests__/DiffusionGitBranchTestCase.php',
     'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php',
+    'DiffusionGitFileContentQueryTestCase' => 'applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php',
     'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php',
     'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php',
     'DiffusionGitResponse' => 'applications/diffusion/response/DiffusionGitResponse.php',
@@ -3105,6 +3106,7 @@ phutil_register_library_map(array(
     'DiffusionFileContentQuery' => 'DiffusionQuery',
     'DiffusionGitBranchTestCase' => 'PhabricatorTestCase',
     'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery',
+    'DiffusionGitFileContentQueryTestCase' => 'PhabricatorTestCase',
     'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery',
     'DiffusionGitRequest' => 'DiffusionRequest',
     'DiffusionGitResponse' => 'AphrontResponse',
diff --git a/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php b/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php
new file mode 100644
index 0000000000..1e69ecd5fa
--- /dev/null
+++ b/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php
@@ -0,0 +1,32 @@
+<?php
+
+final class DiffusionGitFileContentQueryTestCase extends PhabricatorTestCase {
+
+  public function testAuthorName() {
+    // A normal case - no parenthesis in user name
+    $result = DiffusionGitFileContentQuery::match(
+      '8220d5d54f6d5d5552a636576cbe9c35f15b65b2 '.
+      '(Andrew Gallagher       2010-12-03      324) $somevar = $this->call()');
+    $this->assertEqual($result[0], '8220d5d54f6d5d5552a636576cbe9c35f15b65b2');
+    $this->assertEqual($result[1], 'Andrew Gallagher');
+    $this->assertEqual($result[2], ' $somevar = $this->call()');
+
+    // User name like 'Jimmy (He) Zhang'
+    $result = DiffusionGitFileContentQuery::match(
+      '8220d5d54f6d5d5552a636576cbe9c35f15b65b2 '.
+      '( Jimmy (He) Zhang    2013-10-11    5) '.
+      'code(); "(string literal 9999-99-99 2)"; more_code();');
+    $this->assertEqual($result[1], 'Jimmy (He) Zhang');
+    $this->assertEqual($result[2],
+      ' code(); "(string literal 9999-99-99 2)"; more_code();');
+
+    // User name like 'Scott Shapiro (Ads Product Marketing)'
+    $result = DiffusionGitFileContentQuery::match(
+      '8220d5d54f6d5d5552a636576cbe9c35f15b65b2 '.
+      '( Scott Shapiro (Ads Product Marketing)    2013-10-11    5) '.
+      'code(); "(string literal 9999-99-99 2)"; more_code();');
+    $this->assertEqual($result[1], 'Scott Shapiro (Ads Product Marketing)');
+    $this->assertEqual($result[2],
+      ' code(); "(string literal 9999-99-99 2)"; more_code();');
+  }
+}
diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
index 30e10c9177..ec0729d453 100644
--- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
+++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php
@@ -32,6 +32,10 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery {
   }
 
   protected function tokenizeLine($line) {
+    return self::match($line);
+  }
+
+  public static function match($line) {
     $m = array();
     // sample lines:
     //
@@ -41,7 +45,7 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery {
     // 8220d5d54f6d5d5552a636576cbe9c35f15b65b2
     //                       (Andrew Gallagher       2010-12-03      324)
     //                             // Add the lines for trailing context
-    preg_match('/^\s*?(\S+?)\s*\(\s*([^)]*)\s+\d{4}-\d{2}-\d{2}\s+\d+\)(.*)?$/',
+    preg_match('/^\s*?(\S+?)\s*\(\s*(.*?)\s+\d{4}-\d{2}-\d{2}\s+\d+\)(.*)?$/',
                $line, $m);
     $rev_id = $m[1];
     $author = $m[2];