From 1ea9b8b02c19d0621675c6a10292d91f30d8e46d Mon Sep 17 00:00:00 2001 From: vrana Date: Wed, 8 Aug 2012 12:08:41 -0700 Subject: [PATCH] Run lint and unit before updating revision Summary: Depends on D2614. Test Plan: Updated a diff with no lint errors. Updated a diff with lint errors, verified that the previous message is not lost. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3174 --- src/workflow/ArcanistBaseWorkflow.php | 30 +++++ src/workflow/ArcanistDiffWorkflow.php | 178 +++++++++++++++----------- 2 files changed, 133 insertions(+), 75 deletions(-) diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index 4e2b2fb6..52512855 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -1184,6 +1184,22 @@ abstract class ArcanistBaseWorkflow { } + /** + * Try to read a scratch JSON file, if it exists and is readable. + * + * @param string Scratch file name. + * @return array Empty array for failure. + * @task scratch + */ + protected function readScratchJSONFile($path) { + $file = $this->readScratchFile($path); + if (!$file) { + return array(); + } + return json_decode($file, true); + } + + /** * Try to write a scratch file, if there's somewhere to put it and we can * write there. @@ -1201,6 +1217,20 @@ abstract class ArcanistBaseWorkflow { } + /** + * Try to write a scratch JSON file, if there's somewhere to put it and we can + * write there. + * + * @param string Scratch file name to write. + * @param array Data to write. + * @return bool True on success, false on failure. + * @task scratch + */ + protected function writeScratchJSONFile($path, array $data) { + return $this->writeScratchFile($path, json_encode($data)); + } + + /** * Try to remove a scratch file. * diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index e08b7866..256b7676 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -373,7 +373,7 @@ EOTEXT if ($this->getArgument('no-diff')) { $this->removeScratchFile('diff-result.json'); $data = $this->runLintUnit(); - $this->writeScratchFile('diff-result.json', json_encode($data)); + $this->writeScratchJSONFile('diff-result.json', $data); return 0; } @@ -391,17 +391,20 @@ EOTEXT $commit_message = $this->buildCommitMessage(); + if (!$this->shouldOnlyCreateDiff()) { + $revision = $this->buildRevisionFromCommitMessage($commit_message); + } + if ($this->getArgument('background')) { $server = new PhutilConsoleServer(); $server->addExecFutureClient($lint_unit); $server->run(); list($err) = $lint_unit->resolve(); - $data = $this->readScratchFile('diff-result.json'); + $data = $this->readScratchJSONFile('diff-result.json'); if ($err || !$data) { return 1; } - $data = json_decode($data, true); } else { $data = $this->runLintUnit(); } @@ -457,60 +460,20 @@ EOTEXT } } else { - $message = $commit_message; + $revision['diffid'] = $this->getDiffID(); - $revision = array( - 'diffid' => $this->getDiffID(), - 'fields' => $message->getFields(), - ); - - if ($message->getRevisionID()) { - - // With '--verbatim', pass the (possibly modified) local fields. This - // allows the user to edit some fields (like "title" and "summary") - // locally without '--edit' and have changes automatically synchronized. - // Without '--verbatim', we do not update the revision to reflect local - // commit message changes. - if ($this->getArgument('verbatim')) { - $use_fields = $message->getFields(); - } else { - $use_fields = array(); - } - - // TODO: This is silly -- we're getting a text corpus from the server - // and then sending it right back to be parsed. This should be a - // single call. - $remote_corpus = $conduit->callMethodSynchronous( - 'differential.getcommitmessage', - array( - 'revision_id' => $message->getRevisionID(), - 'edit' => 'edit', - 'fields' => $use_fields, - )); - - $should_edit = $this->getArgument('edit'); - if ($should_edit) { - $remote_corpus = $this->newInteractiveEditor($remote_corpus) - ->setName('differential-edit-revision-info') - ->editInteractively(); - } - - $new_message = ArcanistDifferentialCommitMessage::newFromRawCorpus( - $remote_corpus); - - $new_message->pullDataFromConduit($conduit); - $revision['fields'] = $new_message->getFields(); - - $revision['id'] = $message->getRevisionID(); - $this->revisionID = $revision['id']; - - $update_message = $this->getUpdateMessage($revision['fields']); - - $revision['message'] = $update_message; + if ($commit_message->getRevisionID()) { $future = $conduit->callMethod( 'differential.updaterevision', $revision); $result = $future->resolve(); + + foreach (array('edit-messages.json', 'update-messages.json') as $file) { + $messages = $this->readScratchJSONFile($file); + unset($messages[$revision['id']]); + $this->writeScratchJSONFile($file, $messages); + } + echo "Updated an existing Differential revision:\n"; } else { $revision['user'] = $this->getUserPHID(); @@ -623,6 +586,79 @@ EOTEXT } } + private function buildRevisionFromCommitMessage($message) { + $conduit = $this->getConduit(); + + $revision_id = $message->getRevisionID(); + $revision = array( + 'fields' => $message->getFields(), + ); + + if ($revision_id) { + + // With '--verbatim', pass the (possibly modified) local fields. This + // allows the user to edit some fields (like "title" and "summary") + // locally without '--edit' and have changes automatically synchronized. + // Without '--verbatim', we do not update the revision to reflect local + // commit message changes. + if ($this->getArgument('verbatim')) { + $use_fields = $message->getFields(); + } else { + $use_fields = array(); + } + + $should_edit = $this->getArgument('edit'); + $edit_messages = $this->readScratchJSONFile('edit-messages.json'); + $remote_corpus = idx($edit_messages, $revision_id); + + if (!$should_edit || !$remote_corpus || $use_fields) { + $remote_corpus = $conduit->callMethodSynchronous( + 'differential.getcommitmessage', + array( + 'revision_id' => $revision_id, + 'edit' => 'edit', + 'fields' => $use_fields, + )); + } + + if ($should_edit) { + $remote_corpus = $this->newInteractiveEditor($remote_corpus) + ->setName('differential-edit-revision-info') + ->editInteractively(); + $edit_messages[$revision_id] = $remote_corpus; + $this->writeScratchJSONFile('edit-messages.json', $edit_messages); + } + + $new_message = ArcanistDifferentialCommitMessage::newFromRawCorpus( + $remote_corpus); + + $new_message->pullDataFromConduit($conduit); + $revision['fields'] = $new_message->getFields(); + + $revision['id'] = $revision_id; + $this->revisionID = $revision_id; + + $revision['message'] = $this->getArgument('message'); + if (!strlen($revision['message'])) { + $update_messages = $this->readScratchJSONFile('update-messages.json'); + + $update_messages[$revision_id] = $this->getUpdateMessage( + $revision['fields'], + idx($update_messages, $revision_id)); + + $revision['message'] = ArcanistCommentRemover::removeComments( + $update_messages[$revision_id]); + if (!strlen(trim($revision['message']))) { + throw new ArcanistUserAbortException(); + } + + $this->writeScratchJSONFile('update-messages.json', $update_messages); + } + } + + return $revision; + } + protected function shouldOnlyCreateDiff() { if ($this->getArgument('create')) { @@ -1668,12 +1704,7 @@ EOTEXT /** * @task message */ - private function getUpdateMessage(array $fields) { - $comments = $this->getArgument('message'); - if (strlen($comments)) { - return $comments; - } - + private function getUpdateMessage(array $fields, $template = '') { if ($this->getArgument('raw')) { throw new ArcanistUsageException( "When using '--raw' to update a revision, specify an update message ". @@ -1691,29 +1722,26 @@ EOTEXT // ...you shouldn't have to retype the update message. Similar things apply // to Mercurial. - $comments = $this->getDefaultUpdateMessage(); + if ($template == '') { + $comments = $this->getDefaultUpdateMessage(); - $template = - rtrim($comments). - "\n\n". - "# Updating D{$fields['revisionID']}: {$fields['title']}\n". - "#\n". - "# Enter a brief description of the changes included in this update.\n". - "# The first line is used as subject, next lines as comment.\n". - "#\n". - "# If you intended to create a new revision, use:\n". - "# $ arc diff --create\n". - "\n"; + $template = + rtrim($comments). + "\n\n". + "# Updating D{$fields['revisionID']}: {$fields['title']}\n". + "#\n". + "# Enter a brief description of the changes included in this update.\n". + "# The first line is used as subject, next lines as comment.\n". + "#\n". + "# If you intended to create a new revision, use:\n". + "# $ arc diff --create\n". + "\n"; + } $comments = $this->newInteractiveEditor($template) ->setName('differential-update-comments') ->editInteractively(); - $comments = ArcanistCommentRemover::removeComments($comments); - if (!strlen(trim($comments))) { - throw new ArcanistUserAbortException(); - } - return $comments; }