Be more lenient when accepting "Differential Revision" in the presence of custom ad-hoc commit message fields

Summary:
Fixes T8360. We will now parse revisions out of "Differential Revision: X" followed by other ad-hoc fields which we do not recognize. Previously, these fields would be treated as part of the value.

(In the general case, other fields may line wrap so we can't assume that fields are only one line long. However, we can make that assumption safely for this field.)

Also maybe fix whatever was going on in T9965 although that didn't really have a reproduction case.

Test Plan: Added unit tests.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8360

Differential Revision: https://secure.phabricator.com/D17121

src/__phutil_library_map__.php

@@ -381,6 +381,7 @@ phutil_register_library_map(array(
     'DifferentialCloseConduitAPIMethod' => 'applications/differential/conduit/DifferentialCloseConduitAPIMethod.php',
     'DifferentialCommitMessageCustomField' => 'applications/differential/field/DifferentialCommitMessageCustomField.php',
     'DifferentialCommitMessageField' => 'applications/differential/field/DifferentialCommitMessageField.php',
+    'DifferentialCommitMessageFieldTestCase' => 'applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php',
     'DifferentialCommitMessageParser' => 'applications/differential/parser/DifferentialCommitMessageParser.php',
     'DifferentialCommitMessageParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php',
     'DifferentialCommitsField' => 'applications/differential/customfield/DifferentialCommitsField.php',
@@ -5030,6 +5031,7 @@ phutil_register_library_map(array(
     'DifferentialCloseConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialCommitMessageCustomField' => 'DifferentialCommitMessageField',
     'DifferentialCommitMessageField' => 'Phobject',
+    'DifferentialCommitMessageFieldTestCase' => 'PhabricatorTestCase',
     'DifferentialCommitMessageParser' => 'Phobject',
     'DifferentialCommitMessageParserTestCase' => 'PhabricatorTestCase',
     'DifferentialCommitsField' => 'DifferentialCustomField',

src/applications/differential/field/DifferentialRevisionIDCommitMessageField.php

@@ -18,8 +18,25 @@ final class DifferentialRevisionIDCommitMessageField
   }
 
   public function parseFieldValue($value) {
-    // If the value is just "D123" or similar, parse the ID from it directly.
+    // If the complete commit message we are parsing has unrecognized custom
+    // fields at the end, they can end up parsed into the field value for this
+    // field. For example, if the message looks like this:
+
+    // Differential Revision: xyz
+    // Some-Other-Field: abc
+
+    // ...we will receive "xyz\nSome-Other-Field: abc" as the field value for
+    // this field. Ideally, the install would define these fields so they can
+    // parse formally, but we can reasonably assume that only the first line
+    // of any value we encounter actually contains a revision identifier, so
+    // start by throwing away any other lines.
+
     $value = trim($value);
+    $value = phutil_split_lines($value, false);
+    $value = head($value);
+    $value = trim($value);
+
+    // If the value is just "D123" or similar, parse the ID from it directly.
     $matches = null;
     if (preg_match('/^[dD]([1-9]\d*)\z/', $value, $matches)) {
       return (int)$matches[1];

src/applications/differential/field/__tests__/DifferentialCommitMessageFieldTestCase.php

@@ -0,0 +1,30 @@
+<?php
+
+final class DifferentialCommitMessageFieldTestCase
+  extends PhabricatorTestCase {
+
+  public function testRevisionCommitMessageFieldParsing() {
+    $base_uri = 'https://www.example.com/';
+
+    $tests = array(
+      'D123' => 123,
+      'd123' => 123,
+      "  \n  d123 \n " => 123,
+      "D123\nSome-Custom-Field: The End" => 123,
+      "{$base_uri}D123" => 123,
+      "{$base_uri}D123\nSome-Custom-Field: The End" => 123,
+    );
+
+    $env = PhabricatorEnv::beginScopedEnv();
+    $env->overrideEnvConfig('phabricator.base-uri', $base_uri);
+
+    foreach ($tests as $input => $expect) {
+      $actual = id(new DifferentialRevisionIDCommitMessageField())
+        ->parseFieldValue($input);
+      $this->assertEqual($expect, $actual, pht('Parse of: %s', $input));
+    }
+
+    unset($env);
+  }
+
+}

src/applications/differential/parser/DifferentialCommitMessageParser.php

@@ -155,7 +155,10 @@ final class DifferentialCommitMessageParser extends Phobject {
     $field = $key_title;
 
     $seen = array();
-    $lines = explode("\n", trim($corpus));
+
+    $lines = trim($corpus);
+    $lines = phutil_split_lines($lines, false);
+
     $field_map = array();
     foreach ($lines as $key => $line) {
       $match = null;