Perform commit message parsing and construction with new CustomFields

Summary: Ref T2222. Ref T3886. Converts parsing and construction of commit messages to be driven by CustomField.

Test Plan:
This is a huge, messy change. I've made an effort to test it exhasutively, but suspect I probably missed a few behaviors. Roughly:

  - Enumerted all current fields (fields implementing `shouldAppearOnCommitMessage()`) and tried to test them one by one.
  - Used `arc diff --edit` repeatedly to manipulate each field (this workflow hits both the parse and construct steps).
  - Used `arc amend --show` to examine construct output (this does not activate the "edit" mode).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3886, T2222

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

src/__phutil_library_map__.php

@@ -326,6 +326,7 @@ phutil_register_library_map(array(
     'DifferentialArcanistProjectFieldSpecification' => 'applications/differential/field/specification/DifferentialArcanistProjectFieldSpecification.php',
     'DifferentialAsanaRepresentationField' => 'applications/differential/customfield/DifferentialAsanaRepresentationField.php',
     'DifferentialAsanaRepresentationFieldSpecification' => 'applications/differential/field/specification/DifferentialAsanaRepresentationFieldSpecification.php',
+    'DifferentialAuditorsField' => 'applications/differential/customfield/DifferentialAuditorsField.php',
     'DifferentialAuditorsFieldSpecification' => 'applications/differential/field/specification/DifferentialAuditorsFieldSpecification.php',
     'DifferentialAuthorField' => 'applications/differential/customfield/DifferentialAuthorField.php',
     'DifferentialAuthorFieldSpecification' => 'applications/differential/field/specification/DifferentialAuthorFieldSpecification.php',
@@ -360,6 +361,7 @@ phutil_register_library_map(array(
     'DifferentialCommitMessageParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php',
     'DifferentialCommitsField' => 'applications/differential/customfield/DifferentialCommitsField.php',
     'DifferentialCommitsFieldSpecification' => 'applications/differential/field/specification/DifferentialCommitsFieldSpecification.php',
+    'DifferentialConflictsField' => 'applications/differential/customfield/DifferentialConflictsField.php',
     'DifferentialConflictsFieldSpecification' => 'applications/differential/field/specification/DifferentialConflictsFieldSpecification.php',
     'DifferentialController' => 'applications/differential/controller/DifferentialController.php',
     'DifferentialCoreCustomField' => 'applications/differential/customfield/DifferentialCoreCustomField.php',
@@ -403,6 +405,7 @@ phutil_register_library_map(array(
     'DifferentialFieldValidationException' => 'applications/differential/field/exception/DifferentialFieldValidationException.php',
     'DifferentialFreeformFieldSpecification' => 'applications/differential/field/specification/DifferentialFreeformFieldSpecification.php',
     'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
+    'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php',
     'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
     'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
     'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
@@ -454,6 +457,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanField' => 'applications/differential/customfield/DifferentialRevertPlanField.php',
     'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/DifferentialRevertPlanFieldSpecification.php',
     'DifferentialReviewRequestMail' => 'applications/differential/mail/DifferentialReviewRequestMail.php',
+    'DifferentialReviewedByField' => 'applications/differential/customfield/DifferentialReviewedByField.php',
     'DifferentialReviewedByFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewedByFieldSpecification.php',
     'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
     'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
@@ -465,6 +469,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php',
     'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php',
     'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php',
+    'DifferentialRevisionIDField' => 'applications/differential/customfield/DifferentialRevisionIDField.php',
     'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php',
     'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php',
     'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php',
@@ -2898,6 +2903,7 @@ phutil_register_library_map(array(
     'DifferentialArcanistProjectFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialAsanaRepresentationField' => 'DifferentialCustomField',
     'DifferentialAsanaRepresentationFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialAuditorsField' => 'DifferentialStoredCustomField',
     'DifferentialAuditorsFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialAuthorField' => 'DifferentialCustomField',
     'DifferentialAuthorFieldSpecification' => 'DifferentialFieldSpecification',
@@ -2926,6 +2932,7 @@ phutil_register_library_map(array(
     'DifferentialCommitMessageParserTestCase' => 'PhabricatorTestCase',
     'DifferentialCommitsField' => 'DifferentialCustomField',
     'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialConflictsField' => 'DifferentialCustomField',
     'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialController' => 'PhabricatorController',
     'DifferentialCoreCustomField' => 'DifferentialCustomField',
@@ -2971,6 +2978,7 @@ phutil_register_library_map(array(
     'DifferentialFieldSpecificationIncompleteException' => 'Exception',
     'DifferentialFieldValidationException' => 'Exception',
     'DifferentialFreeformFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialGitSVNIDField' => 'DifferentialCustomField',
     'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialHostField' => 'DifferentialCustomField',
     'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification',
@@ -3017,6 +3025,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanField' => 'DifferentialStoredCustomField',
     'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewRequestMail' => 'DifferentialMail',
+    'DifferentialReviewedByField' => 'DifferentialCoreCustomField',
     'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewersField' => 'DifferentialCoreCustomField',
     'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification',
@@ -3035,6 +3044,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
     'DifferentialRevisionEditor' => 'PhabricatorEditor',
+    'DifferentialRevisionIDField' => 'DifferentialCustomField',
     'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase',
     'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialRevisionLandController' => 'DifferentialController',

src/applications/differential/conduit/ConduitAPI_differential_getcommitmessage_Method.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_differential_getcommitmessage_Method
   extends ConduitAPIMethod {
 
@@ -36,91 +33,107 @@ final class ConduitAPI_differential_getcommitmessage_Method
       $revision = id(new DifferentialRevisionQuery())
         ->withIDs(array($id))
         ->setViewer($viewer)
-        ->needRelationships(true)
         ->needReviewerStatus(true)
+        ->needActiveDiffs(true)
         ->executeOne();
-
       if (!$revision) {
         throw new ConduitException('ERR_NOT_FOUND');
       }
     } else {
       $revision = DifferentialRevision::initializeNewRevision($viewer);
+      $revision->attachReviewerStatus(array());
+      $revision->attachActiveDiff(null);
     }
 
-
     $is_edit = $request->getValue('edit');
     $is_create = ($is_edit == 'create');
 
-    $aux_fields = DifferentialFieldSelector::newSelector()
+    $field_list = PhabricatorCustomField::getObjectFields(
-      ->getFieldSpecifications();
-
-    $pro_tips = array();
-
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_field->setUser($viewer);
-      $aux_field->setRevision($revision);
-      $pro_tips[] = $aux_field->getCommitMessageTips();
-      if (!$aux_field->shouldAppearOnCommitMessage()) {
-        unset($aux_fields[$key]);
-      }
-    }
-
-    $aux_fields = DifferentialAuxiliaryField::loadFromStorage(
       $revision,
-      $aux_fields);
+      ($is_edit
-    $aux_fields = mpull($aux_fields, null, 'getCommitMessageKey');
+        ? DifferentialCustomField::ROLE_COMMITMESSAGEEDIT
+        : DifferentialCustomField::ROLE_COMMITMESSAGE));
+
+    $field_list
+      ->setViewer($viewer)
+      ->readFieldsFromStorage($revision);
+
+    $field_map = mpull($field_list->getFields(), null, 'getFieldKeyForConduit');
 
     if ($is_edit) {
-      $fields = $request->getValue('fields');
+      $fields = $request->getValue('fields', array());
-      if (!is_array($fields)) {
-        $fields = array();
-      }
       foreach ($fields as $field => $value) {
-
+        $custom_field = idx($field_map, $field);
-        $aux_field = idx($aux_fields, $field);
+        if (!$custom_field) {
-        if (!$aux_field) {
           throw new Exception(
-            "Commit message includes field '{$field}' which does not ".
+            pht(
-            "correspond to any configured field.");
+              'Commit message includes field "%s", but this field does not '.
+              'correspond to a known field.',
+              $field));
         }
-
         if ($is_create ||
-            $aux_field->shouldOverwriteWhenCommitMessageIsEdited()) {
+            $custom_field->shouldOverwriteWhenCommitMessageIsEdited()) {
-          $aux_field->setValueFromParsedCommitMessage($value);
+          $custom_field->readValueFromCommitMessage($value);
         }
       }
     }
 
-
+    $phids = array();
-    $aux_phids = array();
+    foreach ($field_list->getFields() as $key => $field) {
-    foreach ($aux_fields as $field_key => $field) {
+      $field_phids = $field->getRequiredHandlePHIDsForCommitMessage();
-      $aux_phids[$field_key] = $field->getRequiredHandlePHIDsForCommitMessage();
+      if (!is_array($field_phids)) {
-    }
+        throw new Exception(
-    $phids = array_unique(array_mergev($aux_phids));
+          pht(
-    $handles = id(new PhabricatorHandleQuery())
+            'Custom field "%s" was expected to return an array of handle '.
-      ->setViewer($request->getUser())
+            'PHIDs required for commit message rendering, but returned "%s" '.
-      ->withPHIDs($phids)
+            'instead.',
-      ->execute();
+            $field->getFieldKey(),
-    foreach ($aux_fields as $field_key => $field) {
+            gettype($field_phids)));
-      $field->setHandles(array_select_keys($handles, $aux_phids[$field_key]));
+      }
+      $phids[$key] = $field_phids;
     }
 
+    $all_phids = array_mergev($phids);
+    if ($all_phids) {
+      $all_handles = id(new PhabricatorHandleQuery())
+        ->setViewer($viewer)
+        ->withPHIDs($all_phids)
+        ->execute();
+    } else {
+      $all_handles = array();
+    }
+
+    $key_title = id(new DifferentialTitleField())->getFieldKey();
+    $default_title = DifferentialTitleField::getDefaultTitle();
 
     $commit_message = array();
-    foreach ($aux_fields as $field_key => $field) {
+    foreach ($field_list->getFields() as $key => $field) {
-      $value = $field->renderValueForCommitMessage($is_edit);
+      $handles = array_select_keys($all_handles, $phids[$key]);
-      $label = $field->renderLabelForCommitMessage();
+
+      $label = $field->renderCommitMessageLabel();
+      $value = $field->renderCommitMessageValue($handles);
+
+      if (!is_string($value) && !is_null($value)) {
+        throw new Exception(
+          pht(
+            'Custom field "%s" was expected to render a string or null value, '.
+            'but rendered a "%s" instead.',
+            $field->getFieldKey(),
+            gettype($value)));
+      }
+
+      $is_title = ($key == $key_title);
+
       if (!strlen($value)) {
-        if ($field_key === 'title') {
+        if ($is_title) {
-          $commit_message[] =
+          $commit_message[] = $default_title;
-            DifferentialTitleFieldSpecification::getDefaultRevisionTitle();
         } else {
-          if ($field->shouldAppearOnCommitMessageTemplate() && $is_edit) {
+          if ($is_edit && $field->shouldAppearInCommitMessageTemplate()) {
             $commit_message[] = $label.': ';
           }
         }
       } else {
-        if ($field_key === 'title') {
+        if ($is_title) {
           $commit_message[] = $value;
         } else {
           $value = str_replace(
@@ -137,21 +150,9 @@ final class ConduitAPI_differential_getcommitmessage_Method
     }
 
     if ($is_edit) {
-      $pro_tips = array_mergev($pro_tips);
+      $tip = $this->getProTip($field_list);
-
+      if ($tip !== null) {
-      if (!empty($pro_tips)) {
+        $commit_message[] = "\n".$tip;
-        shuffle($pro_tips);
-        $pro_tip = "Tip: ".$pro_tips[0];
-        $pro_tip = wordwrap($pro_tip, 78, "\n", true);
-
-        $lines = explode("\n", $pro_tip);
-
-        foreach ($lines as $key => $line) {
-          $lines[$key] = "# ".$line;
-        }
-
-        $pro_tip = implode("\n", $lines);
-        $commit_message[] = $pro_tip;
       }
     }
 
@@ -160,4 +161,34 @@ final class ConduitAPI_differential_getcommitmessage_Method
     return $commit_message;
   }
 
+  private function getProTip() {
+    // Any field can provide tips, whether it normally appears on commit
+    // messages or not.
+    $field_list = PhabricatorCustomField::getObjectFields(
+      new DifferentialRevision(),
+      PhabricatorCustomField::ROLE_DEFAULT);
+
+    $tips = array();
+    foreach ($field_list->getFields() as $key => $field) {
+      $tips[] = $field->getProTips();
+    }
+    $tips = array_mergev($tips);
+
+    if (!$tips) {
+      return null;
+    }
+
+    shuffle($tips);
+
+    $tip = pht('Tip: %s', head($tips));
+    $tip = wordwrap($tip, 78, "\n", true);
+
+    $lines = explode("\n", $tip);
+    foreach ($lines as $key => $line) {
+      $lines[$key] = "# ".$line;
+    }
+
+    return implode("\n", $lines);
+  }
+
 }

src/applications/differential/conduit/ConduitAPI_differential_parsecommitmessage_Method.php

@@ -1,15 +1,12 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_differential_parsecommitmessage_Method
   extends ConduitAPIMethod {
 
   private $errors;
 
   public function getMethodDescription() {
-    return "Parse commit messages for Differential fields.";
+    return pht("Parse commit messages for Differential fields.");
   }
 
   public function defineParamTypes() {
@@ -24,91 +21,103 @@ final class ConduitAPI_differential_parsecommitmessage_Method
   }
 
   public function defineErrorTypes() {
-    return array(
+    return array();
-    );
   }
 
   protected function execute(ConduitAPIRequest $request) {
+    $viewer = $request->getUser();
     $corpus = $request->getValue('corpus');
     $is_partial = $request->getValue('partial');
 
-    $aux_fields = DifferentialFieldSelector::newSelector()
+    $revision = new DifferentialRevision();
-      ->getFieldSpecifications();
 
-    foreach ($aux_fields as $key => $aux_field) {
+    $field_list = PhabricatorCustomField::getObjectFields(
-      $aux_field->setUser($request->getUser());
+      $revision,
-      if (!$aux_field->shouldAppearOnCommitMessage()) {
+      DifferentialCustomField::ROLE_COMMITMESSAGE);
-        unset($aux_fields[$key]);
+    $field_list->setViewer($viewer);
-      }
+    $field_map = mpull($field_list->getFields(), null, 'getFieldKeyForConduit');
-    }
-
-    $aux_fields = mpull($aux_fields, null, 'getCommitMessageKey');
 
     $this->errors = array();
 
-    // Build a map from labels (like "Test Plan") to field keys
+    $label_map = $this->buildLabelMap($field_list);
-    // (like "testPlan").
+    $corpus_map = $this->parseCommitMessage($corpus, $label_map);
-    $label_map = $this->buildLabelMap($aux_fields);
+
-    $field_map = $this->parseCommitMessage($corpus, $label_map);
+    $values = array();
+    foreach ($corpus_map as $field_key => $text_value) {
+      $field = idx($field_map, $field_key);
+
+      if (!$field) {
+        throw new Exception(
+          pht(
+            'Parser emitted text value for field key "%s", but no such '.
+            'field exists.',
+            $field_key));
+      }
 
-    $fields = array();
-    foreach ($field_map as $field_key => $field_value) {
-      $field = $aux_fields[$field_key];
       try {
-        $fields[$field_key] = $field->parseValueFromCommitMessage($field_value);
+        $values[$field_key] = $field->parseValueFromCommitMessage($text_value);
-        $field->setValueFromParsedCommitMessage($fields[$field_key]);
       } catch (DifferentialFieldParseException $ex) {
-        $field_label = $field->renderLabelForCommitMessage();
+        $this->errors[] = pht(
-        $this->errors[] =
+          'Error parsing field "%s": %s',
-          "Error parsing field '{$field_label}': ".$ex->getMessage();
+          $field->renderCommitMessageLabel(),
+          $ex->getMessage());
       }
     }
 
     if (!$is_partial) {
-      foreach ($aux_fields as $field_key => $aux_field) {
+      foreach ($field_map as $key => $field) {
         try {
-          $aux_field->validateField();
+          $field->validateCommitMessageValue(idx($values, $key));
         } catch (DifferentialFieldValidationException $ex) {
-          $field_label = $aux_field->renderLabelForCommitMessage();
+          $this->errors[] = pht(
-          $this->errors[] =
+            'Invalid or missing field "%s": %s',
-            "Invalid or missing field '{$field_label}': ".
+            $field->renderCommitMessageLabel(),
-            $ex->getMessage();
+            $ex->getMessage());
         }
       }
     }
 
     return array(
       'errors' => $this->errors,
-      'fields' => $fields,
+      'fields' => $values,
     );
   }
 
-  private function buildLabelMap(array $aux_fields) {
+  private function buildLabelMap(PhabricatorCustomFieldList $field_list) {
-    assert_instances_of($aux_fields, 'DifferentialFieldSpecification');
     $label_map = array();
-    foreach ($aux_fields as $key => $aux_field) {
+
-      $labels = $aux_field->getSupportedCommitMessageLabels();
+    foreach ($field_list->getFields() as $key => $field) {
+      $labels = $field->getCommitMessageLabels();
+      $key = $field->getFieldKeyForConduit();
+
       foreach ($labels as $label) {
         $normal_label = DifferentialCommitMessageParser::normalizeFieldLabel(
           $label);
         if (!empty($label_map[$normal_label])) {
-          $previous = $label_map[$normal_label];
           throw new Exception(
-            "Field label '{$label}' is parsed by two fields: '{$key}' and ".
+            pht(
-            "'{$previous}'. Each label must be parsed by only one field.");
+              'Field label "%s" is parsed by two custom fields: "%s" and '.
+              '"%s". Each label must be parsed by only one field.',
+              $label,
+              $key,
+              $label_map[$normal_label]));
         }
         $label_map[$normal_label] = $key;
       }
     }
+
     return $label_map;
   }
 
 
   private function parseCommitMessage($corpus, array $label_map) {
+    $key_title = id(new DifferentialTitleField())->getFieldKeyForConduit();
+    $key_summary = id(new DifferentialSummaryField())->getFieldKeyForConduit();
+
     $parser = id(new DifferentialCommitMessageParser())
       ->setLabelMap($label_map)
-      ->setTitleKey('title')
+      ->setTitleKey($key_title)
-      ->setSummaryKey('summary');
+      ->setSummaryKey($key_summary);
 
     $result = $parser->parseCorpus($corpus);
 

src/applications/differential/customfield/DifferentialAuditorsField.php

@@ -0,0 +1,53 @@
+<?php
+
+final class DifferentialAuditorsField
+  extends DifferentialStoredCustomField {
+
+  public function getFieldKey() {
+    return 'phabricator:auditors';
+  }
+
+  public function getFieldName() {
+    return pht('Auditors');
+  }
+
+  public function getFieldDescription() {
+    return pht('Allows commits to trigger audits explicitly.');
+  }
+
+  public function getValueForStorage() {
+    return json_encode($this->getValue());
+  }
+
+  public function setValueFromStorage($value) {
+    $this->setValue(phutil_json_decode($value));
+    return $this;
+  }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return true;
+  }
+
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    return nonempty($this->getValue(), array());
+  }
+
+  public function parseCommitMessageValue($value) {
+    return $this->parseObjectList(
+      $value,
+      array(
+        PhabricatorPeoplePHIDTypeUser::TYPECONST,
+        PhabricatorProjectPHIDTypeProject::TYPECONST,
+      ));
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->renderObjectList($handles);
+  }
+
+
+}

src/applications/differential/customfield/DifferentialBlameRevisionField.php

@@ -7,6 +7,10 @@ final class DifferentialBlameRevisionField
     return 'phabricator:blame-revision';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'blameRevision';
+  }
+
   public function getFieldName() {
     return pht('Blame Revision');
   }
@@ -84,4 +88,27 @@ final class DifferentialBlameRevisionField
       $xaction->renderHandleLink($object_phid));
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return true;
+  }
+
+  public function shouldOverwriteWhenCommitMessageIsEdited() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'Blame Revision',
+      'Blame Rev',
+    );
+  }
+
+  public function renderCommitMessageValue() {
+    return $this->getValue();
+  }
+
 }

src/applications/differential/customfield/DifferentialConflictsField.php

@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * This field doesn't do anything, it just parses the "Conflicts:" field which
+ * `git` can insert after a merge, so we don't squish the field value into
+ * some other field.
+ */
+final class DifferentialConflictsField
+  extends DifferentialCustomField {
+
+  public function getFieldKey() {
+    return 'differential:conflicts';
+  }
+
+  public function getFieldKeyForConduit() {
+    return 'conflicts';
+  }
+
+  public function getFieldName() {
+    return pht('Conflicts');
+  }
+
+  public function getFieldDescription() {
+    return pht(
+      'Parses the "Conflicts" field which Git can inject into commit '.
+      'messages.');
+  }
+
+  public function canDisableField() {
+    return false;
+  }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return false;
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return null;
+  }
+
+}

src/applications/differential/customfield/DifferentialCoreCustomField.php

@@ -117,4 +117,13 @@ abstract class DifferentialCoreCustomField
     return $this->value;
   }
 
+  public function readValueFromCommitMessage($value) {
+    $this->setValue($value);
+    return $this;
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->getValue();
+  }
+
 }

src/applications/differential/customfield/DifferentialCustomField.php

@@ -1,10 +1,34 @@
 <?php
 
+/**
+ * @task commitmessage    Integration with Commit Messages
+ */
 abstract class DifferentialCustomField
   extends PhabricatorCustomField {
 
-  public function getRequiredDiffPropertiesForRevisionView() {
+  const ROLE_COMMITMESSAGE      = 'differential:commitmessage';
-    return array();
+  const ROLE_COMMITMESSAGEEDIT  = 'differential:commitmessageedit';
+
+  /**
+   * TODO: It would be nice to remove this, but a lot of different code is
+   * bound together by it. Until everything is modernized, retaining the old
+   * field keys is the only reasonable way to update things one piece
+   * at a time.
+   */
+  public function getFieldKeyForConduit() {
+    return $this->getFieldKey();
+  }
+
+  public function shouldEnableForRole($role) {
+    switch ($role) {
+      case self::ROLE_COMMITMESSAGE:
+        return $this->shouldAppearInCommitMessage();
+      case self::ROLE_COMMITMESSAGEEDIT:
+        return $this->shouldAppearInCommitMessage() &&
+               $this->shouldAllowEditInCommitMessage();
+    }
+
+    return parent::shouldEnableForRole($role);
   }
 
   protected function renderHandleList(array $handles) {
@@ -20,4 +44,172 @@ abstract class DifferentialCustomField
     return phutil_implode_html(phutil_tag('br'), $out);
   }
 
+  public function getRequiredDiffPropertiesForRevisionView() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->getRequiredDiffPropertiesForRevisionView();
+    }
+    return array();
+  }
+
+  protected function parseObjectList($value, array $types) {
+    return id(new PhabricatorObjectListQuery())
+      ->setViewer($this->getViewer())
+      ->setAllowedTypes($types)
+      ->setObjectList($value)
+      ->execute();
+  }
+
+  protected function renderObjectList(array $handles) {
+    if (!$handles) {
+      return null;
+    }
+
+    $out = array();
+    foreach ($handles as $handle) {
+      if ($handle->getPolicyFiltered()) {
+        $out[] = $handle->getPHID();
+      } else if ($handle->isComplete()) {
+        $out[] = $handle->getObjectName();
+      }
+    }
+
+    return implode(', ', $out);
+  }
+
+
+/* -(  Integration with Commit Messages  )----------------------------------- */
+
+
+  /**
+   * @task commitmessage
+   */
+  public function shouldAppearInCommitMessage() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->shouldAppearInCommitMessage();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function shouldAppearInCommitMessageTemplate() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->shouldAppearInCommitMessageTemplate();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function shouldAllowEditInCommitMessage() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->shouldAllowEditInCommitMessage();
+    }
+    return true;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function getProTips() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->getProTips();
+    }
+    return array();
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function getCommitMessageLabels() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->getCommitMessageLabels();
+    }
+    return array($this->renderCommitMessageLabel());
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function parseValueFromCommitMessage($value) {
+    if ($this->getProxy()) {
+      return $this->getProxy()->parseValueFromCommitMessage($value);
+    }
+    return $value;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function readValueFromCommitMessage($value) {
+    if ($this->getProxy()) {
+      $this->getProxy()->readValueFromCommitMessage($value);
+      return $this;
+    }
+    return $this;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function shouldOverwriteWhenCommitMessageIsEdited() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->shouldOverwriteWhenCommitMessageIsEdited();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->getRequiredHandlePHIDsForCommitMessage();
+    }
+    return array();
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function renderCommitMessageLabel() {
+    if ($this->getProxy()) {
+      return $this->getProxy()->renderCommitMessageLabel();
+    }
+    return $this->getFieldName();
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function renderCommitMessageValue(array $handles) {
+    if ($this->getProxy()) {
+      return $this->getProxy()->renderCommitMessageValue($handles);
+    }
+    throw new PhabricatorCustomFieldImplementationIncompleteException($this);
+  }
+
+
+  /**
+   * @task commitmessage
+   */
+  public function validateCommitMessageValue($value) {
+    if ($this->getProxy()) {
+      return $this->getProxy()->validateCommitMessageValue($value);
+    }
+    return;
+  }
+
 }

src/applications/differential/customfield/DifferentialDependsOnField.php

@@ -33,4 +33,12 @@ final class DifferentialDependsOnField
     return $this->renderHandleList($handles);
   }
 
+  public function getProTips() {
+    return array(
+      pht(
+        'Create a dependendency between revisions by writing '.
+        '"Depends on D123" in your summary.'),
+    );
+  }
+
 }

src/applications/differential/customfield/DifferentialGitSVNIDField.php

@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * This field doesn't do anything, it just parses the "git-svn-id" field which
+ * `git svn` inserts into commit messages so that we don't end up mangling
+ * some other field.
+ */
+final class DifferentialGitSVNIDField
+  extends DifferentialCustomField {
+
+  public function getFieldKey() {
+    return 'differential:git-svn-id';
+  }
+
+  public function getFieldKeyForConduit() {
+    return 'gitSVNID';
+  }
+
+  public function getFieldName() {
+    return pht('git-svn-id');
+  }
+
+  public function getFieldDescription() {
+    return pht(
+      'Parses the "git-svn-id" field which Git/SVN can inject into commit '.
+      'messages.');
+  }
+
+  public function canDisableField() {
+    return false;
+  }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return false;
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return null;
+  }
+
+}

src/applications/differential/customfield/DifferentialJIRAIssuesField.php

@@ -9,6 +9,10 @@ final class DifferentialJIRAIssuesField
     return 'phabricator:jira-issues';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'jira.issues';
+  }
+
   public function getValueForStorage() {
     return json_encode($this->getValue());
   }
@@ -232,4 +236,32 @@ final class DifferentialJIRAIssuesField
     $editor->save();
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAppearInCommitMessageTemplate() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'JIRA',
+      'JIRA Issues',
+      'JIRA Issue',
+    );
+  }
+
+  public function parseValueFromCommitMessage($value) {
+    return preg_split('/[\s,]+/', $value, $limit = -1, PREG_SPLIT_NO_EMPTY);
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    $value = $this->getValue();
+    if (!$value) {
+      return null;
+    }
+    return implode(', ', $value);
+  }
+
 }

src/applications/differential/customfield/DifferentialManiphestTasksField.php

@@ -7,6 +7,10 @@ final class DifferentialManiphestTasksField
     return 'differential:maniphest-tasks';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'maniphestTaskPHIDs';
+  }
+
   public function getFieldName() {
     return pht('Maniphest Tasks');
   }
@@ -24,6 +28,10 @@ final class DifferentialManiphestTasksField
   }
 
   public function getRequiredHandlePHIDsForPropertyView() {
+    if (!$this->getObject()->getPHID()) {
+      return array();
+    }
+
     return PhabricatorEdgeQuery::loadDestinationPHIDs(
       $this->getObject()->getPHID(),
       PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK);
@@ -33,4 +41,43 @@ final class DifferentialManiphestTasksField
     return $this->renderHandleList($handles);
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'Maniphest Task',
+      'Maniphest Tasks',
+    );
+  }
+
+  public function parseValueFromCommitMessage($value) {
+    return $this->parseObjectList(
+      $value,
+      array(
+        ManiphestPHIDTypeTask::TYPECONST,
+      ));
+  }
+
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    return $this->getRequiredHandlePHIDsForPropertyView();
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->renderObjectList($handles);
+  }
+
+  public function getProTips() {
+    return array(
+      pht(
+        'Write "Fixes T123" in your summary to automatically close the '.
+        'corresponding task when this change lands.'),
+    );
+  }
+
 }

src/applications/differential/customfield/DifferentialProjectReviewersField.php

@@ -52,4 +52,13 @@ final class DifferentialProjectReviewersField
     }
     return $reviewers;
   }
+
+  public function getProTips() {
+    return array(
+      pht(
+        'You can add a project as a subscriber or reviewer by writing '.
+        '"#projectname" in the appropriate field.'),
+    );
+  }
+
 }

src/applications/differential/customfield/DifferentialRevertPlanField.php

@@ -7,6 +7,10 @@ final class DifferentialRevertPlanField
     return 'phabricator:revert-plan';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'revertPlan';
+  }
+
   public function getFieldName() {
     return pht('Revert Plan');
   }
@@ -131,4 +135,12 @@ final class DifferentialRevertPlanField
     return array($xaction->getNewValue());
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->getValue();
+  }
+
 }

src/applications/differential/customfield/DifferentialReviewedByField.php

@@ -0,0 +1,58 @@
+<?php
+
+final class DifferentialReviewedByField
+  extends DifferentialCoreCustomField {
+
+  public function getFieldKey() {
+    return 'differential:reviewed-by';
+  }
+
+  public function getFieldKeyForConduit() {
+    return 'reviewedByPHIDs';
+  }
+
+  public function getFieldName() {
+    return pht('Reviewed By');
+  }
+
+  public function getFieldDescription() {
+    return pht('Records accepting reviewers in the durable message.');
+  }
+
+  public function shouldAppearInApplicationTransactions() {
+    return false;
+  }
+
+  public function shouldAppearInEditView() {
+    return false;
+  }
+
+  protected function readValueFromRevision(
+    DifferentialRevision $revision) {
+
+    $phids = array();
+    foreach ($revision->getReviewerStatus() as $reviewer) {
+      switch ($reviewer->getStatus()) {
+        case DifferentialReviewerStatus::STATUS_ACCEPTED:
+        case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER:
+          $phids[] = $reviewer->getReviewerPHID();
+          break;
+      }
+    }
+
+    return $phids;
+  }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    return $this->getValue();
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->renderObjectList($handles);
+  }
+
+}

src/applications/differential/customfield/DifferentialReviewersField.php

@@ -7,6 +7,10 @@ final class DifferentialReviewersField
     return 'differential:reviewers';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'reviewerPHIDs';
+  }
+
   public function getFieldName() {
     return pht('Reviewers');
   }
@@ -118,4 +122,47 @@ final class DifferentialReviewersField
     }
     return $reviewers;
   }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAppearInCommitMessageTemplate() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'Reviewer',
+      'Reviewers',
+    );
+  }
+
+  public function parseValueFromCommitMessage($value) {
+    return $this->parseObjectList(
+      $value,
+      array(
+        PhabricatorPeoplePHIDTypeUser::TYPECONST,
+        PhabricatorProjectPHIDTypeProject::TYPECONST,
+      ));
+  }
+
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    return mpull($this->getValue(), 'getReviewerPHID');
+  }
+
+  public function readValueFromCommitMessage($value) {
+    $reviewers = array();
+    foreach ($value as $phid) {
+      $reviewers[] = new DifferentialReviewer($phid, array());
+    }
+    $this->setValue($reviewers);
+
+    return $this;
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->renderObjectList($handles);
+  }
+
 }

src/applications/differential/customfield/DifferentialRevisionIDField.php

@@ -0,0 +1,41 @@
+<?php
+
+final class DifferentialRevisionIDField
+  extends DifferentialCustomField {
+
+  public function getFieldKey() {
+    return 'revisionID';
+  }
+
+  public function getFieldName() {
+    return pht('Differential Revision');
+  }
+
+  public function getFieldDescription() {
+    return pht(
+      'Ties commits to revisions and provides a permananent link between '.
+      'them.');
+  }
+
+  public function canDisableField() {
+    return false;
+  }
+
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return false;
+  }
+
+  public function parseValueFromCommitMessage($value) {
+    return DifferentialRevisionIDFieldSpecification::parseRevisionIDFromURI(
+      $value);
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return PhabricatorEnv::getProductionURI('/D'.$this->getObject()->getID());
+  }
+
+}

src/applications/differential/customfield/DifferentialSubscribersField.php

@@ -7,6 +7,10 @@ final class DifferentialSubscribersField
     return 'differential:subscribers';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'ccPHIDs';
+  }
+
   public function getFieldName() {
     return pht('Subscribers');
   }
@@ -17,6 +21,10 @@ final class DifferentialSubscribersField
 
   protected function readValueFromRevision(
     DifferentialRevision $revision) {
+    if (!$revision->getPHID()) {
+      return array();
+    }
+
     return PhabricatorSubscribersQuery::loadSubscribersForPHID(
       $revision->getPHID());
   }
@@ -46,4 +54,43 @@ final class DifferentialSubscribersField
     return PhabricatorTransactions::TYPE_SUBSCRIBERS;
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAllowEditInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAppearInCommitMessageTemplate() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'CC',
+      'CCs',
+      'Subscriber',
+      'Subscribers',
+    );
+  }
+
+  public function parseValueFromCommitMessage($value) {
+    return $this->parseObjectList(
+      $value,
+      array(
+        PhabricatorPeoplePHIDTypeUser::TYPECONST,
+        PhabricatorProjectPHIDTypeProject::TYPECONST,
+        PhabricatorMailingListPHIDTypeList::TYPECONST,
+      ));
+  }
+
+  public function getRequiredHandlePHIDsForCommitMessage() {
+    return $this->getValue();
+  }
+
+  public function renderCommitMessageValue(array $handles) {
+    return $this->renderObjectList($handles);
+  }
+
 }

src/applications/differential/customfield/DifferentialSummaryField.php

@@ -7,6 +7,10 @@ final class DifferentialSummaryField
     return 'differential:summary';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'summary';
+  }
+
   public function getFieldName() {
     return pht('Summary');
   }
@@ -123,4 +127,16 @@ final class DifferentialSummaryField
     return array($xaction->getNewValue());
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAppearInCommitMessageTemplate() {
+    return true;
+  }
+
+  public function shouldOverwriteWhenCommitMessageIsEdited() {
+    return true;
+  }
+
 }

src/applications/differential/customfield/DifferentialTestPlanField.php

@@ -7,6 +7,10 @@ final class DifferentialTestPlanField
     return 'differential:test-plan';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'testPlan';
+  }
+
   public function getFieldName() {
     return pht('Test Plan');
   }
@@ -36,7 +40,7 @@ final class DifferentialTestPlanField
 
   protected function getCoreFieldRequiredErrorString() {
     return pht(
-      'You must provide a test plan: describe the actions you performed '.
+      'You must provide a test plan. Describe the actions you performed '.
       'to verify the behvaior of this change.');
   }
 
@@ -138,4 +142,33 @@ final class DifferentialTestPlanField
     return array($xaction->getNewValue());
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldAppearInCommitMessageTemplate() {
+    return true;
+  }
+
+  public function shouldOverwriteWhenCommitMessageIsEdited() {
+    return true;
+  }
+
+  public function getCommitMessageLabels() {
+    return array(
+      'Test Plan',
+      'Testplan',
+      'Tested',
+      'Tests',
+    );
+  }
+
+  public function validateCommitMessageValue($value) {
+    if (!strlen($value) && $this->isCoreFieldRequired()) {
+      throw new DifferentialFieldValidationException(
+        $this->getCoreFieldRequiredErrorString());
+    }
+  }
+
+
 }

src/applications/differential/customfield/DifferentialTitleField.php

@@ -7,6 +7,10 @@ final class DifferentialTitleField
     return 'differential:title';
   }
 
+  public function getFieldKeyForConduit() {
+    return 'title';
+  }
+
   public function getFieldName() {
     return pht('Title');
   }
@@ -15,6 +19,10 @@ final class DifferentialTitleField
     return pht('Stores the revision title.');
   }
 
+  public static function getDefaultTitle() {
+    return pht('<<Replace this line with your Revision Title>>');
+  }
+
   protected function readValueFromRevision(
     DifferentialRevision $revision) {
     return $revision->getTitle();
@@ -90,4 +98,29 @@ final class DifferentialTitleField
     }
   }
 
+  public function shouldAppearInCommitMessage() {
+    return true;
+  }
+
+  public function shouldOverwriteWhenCommitMessageIsEdited() {
+    return true;
+  }
+
+  public function validateCommitMessageValue($value) {
+    if (!strlen($value)) {
+      throw new DifferentialFieldValidationException(
+        pht(
+          "You must provide a revision title in the first line ".
+          "of your commit message."));
+    }
+
+    if (preg_match('/^<<.*>>$/', $value)) {
+      throw new DifferentialFieldValidationException(
+        pht(
+          'Replace the line "%s" with a human-readable revision title which '.
+          'describes the changes you are making.',
+          self::getDefaultTitle()));
+    }
+  }
+
 }