Skip to content

Commit

Permalink
Use $EDITOR to prompt users when creating a new commit out of dirty w…
Browse files Browse the repository at this point in the history 
…orking copy changes

Summary:
Fixes T7344.
Currently, we use `phutil_console_prompt()`, which isn't a very good editor. Use the real $EDITOR instead.

100% of the logic here was also a gigantic mess. Clean it up.

Test Plan: Will update in a second with console output from this run.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7344

Differential Revision: https://secure.phabricator.com/D11843
  • Loading branch information
epriestley committed Feb 21, 2015
1 parent 8f8fe44 commit dd59423
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 89 deletions.
28 changes: 22 additions & 6 deletions src/internationalization/ArcanistUSEnglishTranslation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,30 @@ protected function getTranslations() {
'Do you want to mark these files as binary and continue?',
),

'Do you want to amend these %s file(s) to the commit?' => array(
'Do you want to amend this file to the commit?',
'Do you want to amend these files to the commit?',
'Do you want to amend these %s change(s) to the current commit?' => array(
'Do you want to amend this change to the current commit?',
'Do you want to amend these changes to the current commit?',
),

'Do you want to add these %s file(s) to the commit?' => array(
'Do you want to add this file to the commit?',
'Do you want to add these files to the commit?',
'Do you want to create a new commit with these %s change(s)?' => array(
'Do you want to create a new commit with this change?',
'Do you want to create a new commit with these changes?',
),

'(To ignore these %s change(s), add them to ".git/info/exclude".)' =>
array(
'(To ignore this change, add it to ".git/info/exclude".)',
'(To ignore these changes, add themto ".git/info/exclude".)',
),

'(To ignore these %s change(s), add them to "svn:ignore".)' => array(
'(To ignore this change, add it to "svn:ignore".)',
'(To ignore these changes, add them to "svn:ignore".)',
),

'(To ignore these %s change(s), add them to ".hgignore".)' => array(
'(To ignore this change, add it to ".hgignore".)',
'(To ignore these changes, add them to ".hgignore".)',
),

'%s line(s)' => array('line', 'lines'),
Expand Down
6 changes: 0 additions & 6 deletions src/workflow/ArcanistDiffWorkflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1923,9 +1923,6 @@ private function parseCommitMessagesIntoFields(array $local) {
$messages = array();
foreach ($local as $hash => $info) {
$text = $info['message'];
if (trim($text) == self::AUTO_COMMIT_TITLE) {
continue;
}
$obj = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
$messages[$hash] = $obj;
}
Expand Down Expand Up @@ -2162,9 +2159,6 @@ private function formatUsableLogs(array $usable) {
foreach ($usable as $message) {
// Pick the first line out of each message.
$text = trim($message);
if ($text == self::AUTO_COMMIT_TITLE) {
continue;
}
$text = head(explode("\n", $text));
$default[] = ' - '.$text."\n";
}
Expand Down
193 changes: 116 additions & 77 deletions src/workflow/ArcanistWorkflow.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ abstract class ArcanistWorkflow extends Phobject {
const COMMIT_ALLOW = 1;
const COMMIT_ENABLE = 2;

const AUTO_COMMIT_TITLE = 'Automatic commit by arc';

private $commitMode = self::COMMIT_DISABLE;

private $conduit;
Expand Down Expand Up @@ -837,45 +835,6 @@ final public function requireCleanWorkingCopy() {
" Working copy: __%s__\n\n",
$api->getPath());

$untracked = $api->getUntrackedChanges();
if ($this->shouldRequireCleanUntrackedFiles()) {

if (!empty($untracked)) {
echo "You have untracked files in this working copy.\n\n".
$working_copy_desc.
" Untracked files in working copy:\n".
" ".implode("\n ", $untracked)."\n\n";

if ($api instanceof ArcanistGitAPI) {
echo phutil_console_wrap(
"Since you don't have '.gitignore' rules for these files and have ".
"not listed them in '.git/info/exclude', you may have forgotten ".
"to 'git add' them to your commit.\n");
} else if ($api instanceof ArcanistSubversionAPI) {
echo phutil_console_wrap(
"Since you don't have 'svn:ignore' rules for these files, you may ".
"have forgotten to 'svn add' them.\n");
} else if ($api instanceof ArcanistMercurialAPI) {
echo phutil_console_wrap(
"Since you don't have '.hgignore' rules for these files, you ".
"may have forgotten to 'hg add' them to your commit.\n");
}

if ($this->askForAdd($untracked)) {
$api->addToCommit($untracked);
$must_commit += array_flip($untracked);
} else if ($this->commitMode == self::COMMIT_DISABLE) {
$prompt = $this->getAskForAddPrompt($untracked);
if (phutil_console_confirm($prompt)) {
throw new ArcanistUsageException(pht(
"Add these files and then run 'arc %s' again.",
$this->getWorkflowName()));
}
}

}
}

// NOTE: this is a subversion-only concept.
$incomplete = $api->getIncompleteChanges();
if ($incomplete) {
Expand Down Expand Up @@ -910,15 +869,75 @@ final public function requireCleanWorkingCopy() {
" ".implode("\n ", $missing)));
}

$uncommitted = $api->getUncommittedChanges();
$unstaged = $api->getUnstagedChanges();
if ($unstaged) {
echo "You have unstaged changes in this working copy.\n\n".
$working_copy_desc.
" Unstaged changes in working copy:\n".
" ".implode("\n ", $unstaged)."\n";
if ($this->askForAdd($unstaged)) {
$api->addToCommit($unstaged);
$must_commit += array_flip($unstaged);

// We only want files which are purely uncommitted.
$uncommitted = array_diff($uncommitted, $unstaged);

$untracked = $api->getUntrackedChanges();
if (!$this->shouldRequireCleanUntrackedFiles()) {
$untracked = array();
}

$should_commit = false;
if ($unstaged || $uncommitted) {

// NOTE: We're running this because it builds a cache and can take a
// perceptible amount of time to arrive at an answer, but we don't want
// to pause in the middle of printing the output below.
$this->getShouldAmend();

echo pht(
"You have uncommitted changes in this working copy.\n\n%s",
$working_copy_desc);

$lists = array();

if ($untracked) {
if ($api instanceof ArcanistGitAPI) {
$hint = pht(
'(To ignore these %s change(s), add them to ".git/info/exclude".)',
new PhutilNumber(count($untracked)));
} else if ($api instanceof ArcanistSubversionAPI) {
$hint = pht(
'(To ignore these %s change(s), add them to "svn:ignore".)',
new PhutilNumber(count($untracked)));
} else if ($api instanceof ArcanistMercurialAPI) {
$hint = pht(
'(To ignore these %s change(s), add them to ".hgignore".)',
new PhutilNumber(count($untracked)));
}

$untracked_list = " ".implode("\n ", $untracked);
$lists[] = pht(
" Untracked changes in working copy:\n %s\n%s",
$hint,
$untracked_list);
}

if ($unstaged) {
$unstaged_list = " ".implode("\n ", $unstaged);
$lists[] = pht(
" Unstaged changes in working copy:\n%s",
$unstaged_list);
}

if ($uncommitted) {
$uncommitted_list = " ".implode("\n ", $uncommitted);
$lists[] = pht(
" Uncommitted changes in working copy:\n%s",
$uncommitted_list);
}

echo implode("\n\n", $lists);

$all_uncommitted = array_merge($unstaged, $uncommitted);
if ($this->askForAdd($all_uncommitted)) {
if ($unstaged) {
$api->addToCommit($unstaged);
}
$should_commit = true;
} else {
$permit_autostash = $this->getConfigFromAnySource(
'arc.autostash',
Expand All @@ -929,40 +948,59 @@ final public function requireCleanWorkingCopy() {
$api->stashChanges();
$this->stashed = true;
} else {
throw new ArcanistUsageException(
'Stage and commit (or revert) them before proceeding.');
if ($untracked && !$unstaged && !$uncommitted) {
// Give a tailored message if there are only untracked files,
// because the advice to commit files does not make sense in
// Subversion.
throw new ArcanistUsageException(
pht(
'You can not continue with untracked changes. Add them, '.
'discard them, or mark them as ignored before proceeding.'));
} else {
throw new ArcanistUsageException(
pht(
'You can not continue with uncommitted changes. Commit '.
'or discard them before proceeding.'));
}
}
}
}

$uncommitted = $api->getUncommittedChanges();
foreach ($uncommitted as $key => $path) {
if (array_key_exists($path, $must_commit)) {
unset($uncommitted[$key]);
}
}
if ($uncommitted) {
echo "You have uncommitted changes in this working copy.\n\n".
$working_copy_desc.
" Uncommitted changes in working copy:\n".
" ".implode("\n ", $uncommitted)."\n";
if ($this->askForAdd($uncommitted)) {
$must_commit += array_flip($uncommitted);
} else {
throw new ArcanistUncommittedChangesException(
'Commit (or revert) them before proceeding.');
}
}

if ($must_commit) {
if ($should_commit) {
if ($this->getShouldAmend()) {
$commit = head($api->getLocalCommitInformation());
$api->amendCommit($commit['message']);
} else if ($api->supportsLocalCommits()) {
$commit_message = phutil_console_prompt('Enter commit message:');
if ($commit_message == '') {
$commit_message = self::AUTO_COMMIT_TITLE;
$template =
"\n\n".
"# ".pht('Enter a commit message.')."\n#\n".
"# ".pht('Changes:')."\n#\n";

$paths = array_merge($uncommitted, $unstaged);
$paths = array_unique($paths);
sort($paths);

foreach ($paths as $path) {
$template .= "# ".$path."\n";
}

$commit_message = $this->newInteractiveEditor($template)
->setName(pht('commit-message'))
->editInteractively();

if ($commit_message === $template) {
throw new ArcanistUsageException(
pht('You must provide a commit message.'));
}

$commit_message = ArcanistCommentRemover::removeComments(
$commit_message);

if (!strlen($commit_message)) {
throw new ArcanistUsageException(
pht('You must provide a nonempty commit message.'));
}

$api->doCommit($commit_message);
}
}
Expand Down Expand Up @@ -1005,6 +1043,7 @@ private function calculateShouldAmend() {

// TODO: Check commits since tracking branch. If empty then return false.

// Don't amend the current commit if it has already been published.
$repository = $this->loadProjectRepository();
if ($repository) {
$callsign = $repository['callsign'];
Expand All @@ -1013,7 +1052,7 @@ private function calculateShouldAmend() {
'diffusion.querycommits',
array('names' => array($commit_name)));
$known_commit = idx($result['identifierMap'], $commit_name);
if (!$known_commit) {
if ($known_commit) {
return false;
}
}
Expand Down Expand Up @@ -1049,11 +1088,11 @@ private function askForAdd(array $files) {
private function getAskForAddPrompt(array $files) {
if ($this->getShouldAmend()) {
$prompt = pht(
'Do you want to amend these %s file(s) to the commit?',
'Do you want to amend these %s change(s) to the current commit?',
new PhutilNumber(count($files)));
} else {
$prompt = pht(
'Do you want to add these %s file(s) to the commit?',
'Do you want to create a new commit with these %s change(s)?',
new PhutilNumber(count($files)));
}
return $prompt;
Expand Down

0 comments on commit dd59423

Please sign in to comment.