Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -356,6 +356,8 @@ 'DifferentialCommentPreviewController' => 'applications/differential/controller/DifferentialCommentPreviewController.php', 'DifferentialCommentQuery' => 'applications/differential/query/DifferentialCommentQuery.php', 'DifferentialCommentSaveController' => 'applications/differential/controller/DifferentialCommentSaveController.php', + 'DifferentialCommitMessageParser' => 'applications/differential/parser/DifferentialCommitMessageParser.php', + 'DifferentialCommitMessageParserTestCase' => 'applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php', 'DifferentialCommitsField' => 'applications/differential/customfield/DifferentialCommitsField.php', 'DifferentialCommitsFieldSpecification' => 'applications/differential/field/specification/DifferentialCommitsFieldSpecification.php', 'DifferentialConflictsFieldSpecification' => 'applications/differential/field/specification/DifferentialConflictsFieldSpecification.php', @@ -2919,6 +2921,7 @@ 'DifferentialCommentPreviewController' => 'DifferentialController', 'DifferentialCommentQuery' => 'PhabricatorOffsetPagedQuery', 'DifferentialCommentSaveController' => 'DifferentialController', + 'DifferentialCommitMessageParserTestCase' => 'PhabricatorTestCase', 'DifferentialCommitsField' => 'DifferentialCustomField', 'DifferentialCommitsFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialConflictsFieldSpecification' => 'DifferentialFieldSpecification', Index: src/applications/differential/conduit/ConduitAPI_differential_parsecommitmessage_Method.php =================================================================== --- src/applications/differential/conduit/ConduitAPI_differential_parsecommitmessage_Method.php +++ src/applications/differential/conduit/ConduitAPI_differential_parsecommitmessage_Method.php @@ -89,7 +89,8 @@ foreach ($aux_fields as $key => $aux_field) { $labels = $aux_field->getSupportedCommitMessageLabels(); foreach ($labels as $label) { - $normal_label = strtolower($label); + $normal_label = DifferentialCommitMessageParser::normalizeFieldLabel( + $label); if (!empty($label_map[$normal_label])) { $previous = $label_map[$normal_label]; throw new Exception( @@ -102,106 +103,20 @@ return $label_map; } - private function buildLabelRegexp(array $label_map) { - $field_labels = array_keys($label_map); - foreach ($field_labels as $key => $label) { - $field_labels[$key] = preg_quote($label, '/'); - } - $field_labels = implode('|', $field_labels); - - $field_pattern = '/^(?P'.$field_labels.'):(?P.*)$/i'; - - return $field_pattern; - } private function parseCommitMessage($corpus, array $label_map) { - $label_regexp = $this->buildLabelRegexp($label_map); - - // Note, deliberately not populating $seen with 'title' because it is - // optional to include the 'Title:' label. We're doing a little special - // casing to consume the first line as the title regardless of whether you - // label it as such or not. - $field = 'title'; - - $seen = array(); - $lines = explode("\n", trim($corpus)); - $field_map = array(); - foreach ($lines as $key => $line) { - $match = null; - if (preg_match($label_regexp, $line, $match)) { - $lines[$key] = trim($match['text']); - $field = $label_map[strtolower($match['field'])]; - if (!empty($seen[$field])) { - $this->errors[] = "Field '{$field}' occurs twice in commit message!"; - } - $seen[$field] = true; - } - $field_map[$key] = $field; - } + $parser = id(new DifferentialCommitMessageParser()) + ->setLabelMap($label_map) + ->setTitleKey('title') + ->setSummaryKey('summary'); - $fields = array(); - foreach ($lines as $key => $line) { - $fields[$field_map[$key]][] = $line; - } + $result = $parser->parseCorpus($corpus); - // This is a piece of special-cased magic which allows you to omit the - // field labels for "title" and "summary". If the user enters a large block - // of text at the beginning of the commit message with an empty line in it, - // treat everything before the blank line as "title" and everything after - // as "summary". - if (isset($fields['title']) && empty($fields['summary'])) { - $lines = $fields['title']; - for ($ii = 0; $ii < count($lines); $ii++) { - if (strlen(trim($lines[$ii])) == 0) { - break; - } - } - if ($ii != count($lines)) { - $fields['title'] = array_slice($lines, 0, $ii); - $fields['summary'] = array_slice($lines, $ii); - } - } - - // Implode all the lines back into chunks of text. - foreach ($fields as $name => $lines) { - $data = rtrim(implode("\n", $lines)); - $data = ltrim($data, "\n"); - $fields[$name] = $data; - } - - // This is another piece of special-cased magic which allows you to - // enter a ridiculously long title, or just type a big block of stream - // of consciousness text, and have some sort of reasonable result conjured - // from it. - if (isset($fields['title'])) { - $terminal = '...'; - $title = $fields['title']; - $short = phutil_utf8_shorten($title, 250, $terminal); - if ($short != $title) { - - // If we shortened the title, split the rest into the summary, so - // we end up with a title like: - // - // Title title tile title title... - // - // ...and a summary like: - // - // ...title title title. - // - // Summary summary summary summary. - - $summary = idx($fields, 'summary', ''); - $offset = strlen($short) - strlen($terminal); - $remainder = ltrim(substr($fields['title'], $offset)); - $summary = '...'.$remainder."\n\n".$summary; - $summary = rtrim($summary, "\n"); - - $fields['title'] = $short; - $fields['summary'] = $summary; - } + foreach ($parser->getErrors() as $error) { + $this->errors[] = $error; } - return $fields; + return $result; } Index: src/applications/differential/parser/DifferentialCommitMessageParser.php =================================================================== --- /dev/null +++ src/applications/differential/parser/DifferentialCommitMessageParser.php @@ -0,0 +1,210 @@ +setLabelMap($label_map) + * ->setTitleKey($key_title) + * ->setSummaryKey($key_summary); + * + * $fields = $parser->parseCorpus($corpus); + * $errors = $parser->getErrors(); + * + * This is used by Differential to parse messages entered from the command line. + * + * @task config Configuring the Parser + * @task parse Parsing Messages + * @task support Support Methods + * @task internal Internals + */ +final class DifferentialCommitMessageParser { + + private $labelMap; + private $titleKey; + private $summaryKey; + private $errors; + + +/* -( Configuring the Parser )--------------------------------------------- */ + + + /** + * @task config + */ + public function setLabelMap(array $label_map) { + $this->labelMap = $label_map; + return $this; + } + + + /** + * @task config + */ + public function setTitleKey($title_key) { + $this->titleKey = $title_key; + return $this; + } + + + /** + * @task config + */ + public function setSummaryKey($summary_key) { + $this->summaryKey = $summary_key; + return $this; + } + + +/* -( Parsing Messages )--------------------------------------------------- */ + + + /** + * @task parse + */ + public function parseCorpus($corpus) { + $this->errors = array(); + + $label_map = $this->labelMap; + $key_title = $this->titleKey; + $key_summary = $this->summaryKey; + + if (!$key_title || !$key_summary || ($label_map === null)) { + throw new Exception( + pht( + 'Expected labelMap, summaryKey and titleKey to be set before '. + 'parsing a corpus.')); + } + + $label_regexp = $this->buildLabelRegexp($label_map); + + // NOTE: We're special casing things here to make the "Title:" label + // optional in the message. + $field = $key_title; + + $seen = array(); + $lines = explode("\n", trim($corpus)); + $field_map = array(); + foreach ($lines as $key => $line) { + $match = null; + if (preg_match($label_regexp, $line, $match)) { + $lines[$key] = trim($match['text']); + $field = $label_map[self::normalizeFieldLabel($match['field'])]; + if (!empty($seen[$field])) { + $this->errors[] = pht( + 'Field "%s" occurs twice in commit message!', + $field); + } + $seen[$field] = true; + } + $field_map[$key] = $field; + } + + $fields = array(); + foreach ($lines as $key => $line) { + $fields[$field_map[$key]][] = $line; + } + + // This is a piece of special-cased magic which allows you to omit the + // field labels for "title" and "summary". If the user enters a large block + // of text at the beginning of the commit message with an empty line in it, + // treat everything before the blank line as "title" and everything after + // as "summary". + if (isset($fields[$key_title]) && empty($fields[$key_summary])) { + $lines = $fields[$key_title]; + for ($ii = 0; $ii < count($lines); $ii++) { + if (strlen(trim($lines[$ii])) == 0) { + break; + } + } + if ($ii != count($lines)) { + $fields[$key_title] = array_slice($lines, 0, $ii); + $summary = array_slice($lines, $ii); + if (strlen(trim(implode("\n", $summary)))) { + $fields[$key_summary] = $summary; + } + } + } + + // Implode all the lines back into chunks of text. + foreach ($fields as $name => $lines) { + $data = rtrim(implode("\n", $lines)); + $data = ltrim($data, "\n"); + $fields[$name] = $data; + } + + // This is another piece of special-cased magic which allows you to + // enter a ridiculously long title, or just type a big block of stream + // of consciousness text, and have some sort of reasonable result conjured + // from it. + if (isset($fields[$key_title])) { + $terminal = '...'; + $title = $fields[$key_title]; + $short = phutil_utf8_shorten($title, 250, $terminal); + if ($short != $title) { + + // If we shortened the title, split the rest into the summary, so + // we end up with a title like: + // + // Title title tile title title... + // + // ...and a summary like: + // + // ...title title title. + // + // Summary summary summary summary. + + $summary = idx($fields, $key_summary, ''); + $offset = strlen($short) - strlen($terminal); + $remainder = ltrim(substr($fields[$key_title], $offset)); + $summary = '...'.$remainder."\n\n".$summary; + $summary = rtrim($summary, "\n"); + + $fields[$key_title] = $short; + $fields[$key_summary] = $summary; + } + } + + return $fields; + } + + + /** + * @task parse + */ + public function getErrors() { + return $this->errors; + } + + +/* -( Support Methods )---------------------------------------------------- */ + + + /** + * @task support + */ + public static function normalizeFieldLabel($label) { + return phutil_utf8_strtolower($label); + } + + +/* -( Internals )---------------------------------------------------------- */ + + + /** + * @task internal + */ + private function buildLabelRegexp(array $label_map) { + $field_labels = array_keys($label_map); + foreach ($field_labels as $key => $label) { + $field_labels[$key] = preg_quote($label, '/'); + } + $field_labels = implode('|', $field_labels); + + $field_pattern = '/^(?P'.$field_labels.'):(?P.*)$/i'; + + return $field_pattern; + } + +} Index: src/applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/DifferentialCommitMessageParserTestCase.php @@ -0,0 +1,65 @@ +setLabelMap($fields) + ->setTitleKey('title') + ->setSummaryKey('summary'); + + $result_output = $parser->parseCorpus($message); + $result_errors = $parser->getErrors(); + + $this->assertEqual($output, $result_output); + $this->assertEqual($errors, $result_errors); + } + } + + public function testDifferentialCommitMessageParserNormalization() { + $map = array( + 'Test Plan' => 'test plan', + 'REVIEWERS' => 'reviewers', + 'sUmmArY' => 'summary', + ); + + foreach ($map as $input => $expect) { + $this->assertEqual( + $expect, + DifferentialCommitMessageParser::normalizeFieldLabel($input), + pht('Field normalization of label "%s".', $input)); + } + } + +} Index: src/applications/differential/parser/__tests__/messages/double-field.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/double-field.txt @@ -0,0 +1,17 @@ +fix bug + +color: red +color: blue +~~~~~~~~~~ +{ + "color": "color" +} +~~~~~~~~~~ +{ + "title": "fix bug", + "color": "red\nblue" +} +~~~~~~~~~~ +[ + "Field \"color\" occurs twice in commit message!" +] Index: src/applications/differential/parser/__tests__/messages/long-title.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/long-title.txt @@ -0,0 +1,11 @@ +123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 + +~~~~~~~~~~ +{} +~~~~~~~~~~ +{ + "title": "1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567...", + "summary": "...89012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890" +} +~~~~~~~~~~ +[] Index: src/applications/differential/parser/__tests__/messages/multi-label.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/multi-label.txt @@ -0,0 +1,15 @@ +fix bug + +hue: black +~~~~~~~~~~ +{ + "color": "color", + "hue": "color" +} +~~~~~~~~~~ +{ + "title": "fix bug", + "color": "black" +} +~~~~~~~~~~ +[] Index: src/applications/differential/parser/__tests__/messages/normal.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/normal.txt @@ -0,0 +1,24 @@ +Title + +Summary: This is the summary. + +Test Plan: Tested things. + +Reviewers: alincoln + +~~~~~~~~~~ +{ + "title": "title", + "summary": "summary", + "test plan": "test plan", + "reviewers": "reviewers" +} +~~~~~~~~~~ +{ + "title": "Title", + "summary": "This is the summary.", + "test plan": "Tested things.", + "reviewers": "alincoln" +} +~~~~~~~~~~ +[] Index: src/applications/differential/parser/__tests__/messages/simple.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/simple.txt @@ -0,0 +1,12 @@ +fix bug + +it had a bug but I fixed it +~~~~~~~~~~ +{} +~~~~~~~~~~ +{ + "title": "fix bug", + "summary": "it had a bug but I fixed it" +} +~~~~~~~~~~ +[] Index: src/applications/differential/parser/__tests__/messages/trivial.txt =================================================================== --- /dev/null +++ src/applications/differential/parser/__tests__/messages/trivial.txt @@ -0,0 +1,9 @@ +fix bug +~~~~~~~~~~ +{} +~~~~~~~~~~ +{ + "title": "fix bug" +} +~~~~~~~~~~ +[]