revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00
Code Issues Releases Wiki Activity

Improve 'arc' behavior under git mutable history with ambiguous commit messages

Browse source 
Summary:
Currently, we throw a fairly perplexing error when there are multiple valid
commit messages. Installs can also remove the "test plan" field entirely, which
is the only really strong discriminator here.

When the message to use is ambiguous, show the user all the valid messages and
prompt them to choose one.

Also add a -C flag like "git commit -C", so they can choose a message
explicitly.

Test Plan: Ran "arc diff HEAD^^^^^", "arc diff -C <rev>".

Reviewers: cpiro, btrahan, jungejason

Reviewed By: cpiro

CC: aran, cpiro

Differential Revision: https://secure.phabricator.com/D1385
This commit is contained in:
epriestley 2012-01-12 19:03:11 -08:00
parent 93db641a3e
commit f271e1d8e8
3 changed files with 86 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/repository/api/base/ArcanistRepositoryAPI.php
View file

@ -155,6 +155,10 @@ abstract class ArcanistRepositoryAPI {
abstract public function supportsRelativeLocalCommits();
abstract public function getWorkingCopyRevision();
public function getCommitMessageForRevision($revision) {
throw new ArcanistCapabilityNotSupportedException($this);
}
public function parseRelativeLocalCommit(array $argv) {
throw new ArcanistCapabilityNotSupportedException($this);
}

9
src/repository/api/git/ArcanistGitAPI.php
View file

@ -610,4 +610,13 @@ class ArcanistGitAPI extends ArcanistRepositoryAPI {
"'git push', or 'git svn dcommit', or by printing and faxing it).";
}
public function getCommitMessageForRevision($rev) {
list($message) = execx(
'(cd %s && git log -n1 %s)',
$this->getPath(),
$rev);
$parser = new ArcanistDiffParser();
return head($parser->parseDiff($message));
}
}

88
src/workflow/diff/ArcanistDiffWorkflow.php
View file

@ -94,6 +94,20 @@ EOTEXT
'help' => 'When creating a revision, read revision information '.
'from this file.',
),
'use-commit-message' => array(
'supports' => array(
'git',
// TODO: Support mercurial.
),
'short' => 'C',
'param' => 'commit',
'help' => 'Read revision information from a specific commit.',
'conflicts' => array(
'only' => null,
'preview' => null,
'update' => null,
),
),
'edit' => array(
'supports' => array(
'git',
@ -144,7 +158,6 @@ EOTEXT
'edit' => '--create can not be used with --edit.',
'only' => '--create can not be used with --only.',
'preview' => '--create can not be used with --preview.',
'update' => '--create can not be used with --update.',
),
),
@ -469,6 +482,10 @@ EOTEXT
return false;
}
if ($this->getArgument('use-commit-message')) {
return false;
}
if ($this->isRawDiffSource()) {
return true;
}
@ -1073,6 +1090,11 @@ EOTEXT
$is_create = $this->getArgument('create');
$is_update = $this->getArgument('update');
$is_raw = $this->isRawDiffSource();
$is_message = $this->getArgument('use-commit-message');
if ($is_message) {
return $this->getCommitMessageFromCommit($is_message);
}
$message = null;
if ($is_create) {
@ -1100,6 +1122,19 @@ EOTEXT
}
/**
* @task message
*/
private function getCommitMessageFromCommit($rev) {
$change = $this->getRepositoryAPI()->getCommitMessageForRevision($rev);
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
$change->getMetadata('message'));
$message->pullDataFromConduit($this->getConduit());
$this->validateCommitMessage($message);
return $message;
}
/**
* @task message
*/
@ -1287,8 +1322,10 @@ EOTEXT
$problems = array();
$parsed = array();
$hashes = array();
foreach ($commit_messages as $key => $change) {
$problems[$key] = array();
$hashes[$key] = $change->getCommitHash();
try {
$message = ArcanistDifferentialCommitMessage::newFromRawCorpus(
@ -1305,26 +1342,47 @@ EOTEXT
}
}
$blessed = null;
$revision_id = -1;
$valid = array();
foreach ($problems as $key => $problem_list) {
if ($problem_list) {
continue;
}
if ($revision_id === -1) {
$revision_id = $parsed[$key]->getRevisionID();
$blessed = $parsed[$key];
} else {
throw new ArcanistUsageException(
"Changes in the specified commit range include more than one ".
"commit with a valid template commit message. This is ambiguous, ".
"your commit range should contain only one template commit ".
"message. Alternatively, use --preview to ignore commit ".
"messages.");
}
$valid[$key] = $parsed[$key];
}
if ($revision_id === -1) {
$blessed = null;
if (count($valid) == 1) {
$blessed = head($valid);
} else if (count($valid) > 1) {
echo phutil_console_wrap(
"Changes in the specified commit range include more than one commit ".
"with a valid template commit message. Choose the message you want ".
"to use (you can also use the -C flag).\n\n");
foreach ($valid as $key => $message) {
$hash = substr($hashes[$key], 0, 7);
$title = $commit_messages[$key]->getMetadata('message');
$title = head(explode("\n", trim($title)));
$title = phutil_utf8_shorten($title, 64);
echo " {$hash} {$title}\n";
}
echo " none Edit a blank template.";
do {
$choose = phutil_console_prompt('Use which commit message [none]?');
if ($choose == 'none' || $choose == '') {
return $this->getCommitMessageFromUser();
} else {
foreach ($valid as $key => $message) {
if (!strncmp($hashes[$key], $choose, strlen($choose))) {
$blessed = $valid[$key];
break;
}
}
}
} while (!$blessed);
}
if (!$blessed) {
$desc = implode("\n", array_mergev($problems));
if (count($problems) > 1) {
throw new ArcanistUsageException(