Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15419140
D20985.id50003.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
13 KB
Referenced Files
None
Subscribers
None
D20985.id50003.diff
View Options
diff --git a/src/differential/ArcanistDifferentialCommitMessage.php b/src/differential/ArcanistDifferentialCommitMessage.php
--- a/src/differential/ArcanistDifferentialCommitMessage.php
+++ b/src/differential/ArcanistDifferentialCommitMessage.php
@@ -8,6 +8,7 @@
private $rawCorpus;
private $revisionID;
private $fields = array();
+ private $xactions = null;
private $gitSVNBaseRevision;
private $gitSVNBasePath;
@@ -37,6 +38,13 @@
return $this->revisionID;
}
+ public function getRevisionMonogram() {
+ if ($this->revisionID) {
+ return 'D'.$this->revisionID;
+ }
+ return null;
+ }
+
public function pullDataFromConduit(
ConduitClient $conduit,
$partial = false) {
@@ -50,6 +58,9 @@
$this->fields = $result['fields'];
+ // NOTE: This does not exist prior to late October 2017.
+ $this->xactions = idx($result, 'transactions');
+
if (!empty($result['errors'])) {
throw new ArcanistDifferentialCommitMessageParserException(
$result['errors']);
@@ -93,6 +104,10 @@
return md5($fields);
}
+ public function getTransactions() {
+ return $this->xactions;
+ }
+
/**
* Extract the revision ID from a commit message.
*
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -21,6 +21,8 @@
private $diffPropertyFutures = array();
private $commitMessageFromRevision;
private $hitAutotargets;
+ private $revisionTransactions;
+ private $revisionIsDraft;
const STAGING_PUSHED = 'pushed';
const STAGING_USER_SKIP = 'user.skip';
@@ -113,8 +115,7 @@
'param' => 'commit',
'help' => pht('Read revision information from a specific commit.'),
'conflicts' => array(
- 'only' => null,
- 'preview' => null,
+ 'only' => null,
'update' => null,
),
),
@@ -176,14 +177,10 @@
'%s can not be used with %s.',
'--create',
'--edit'),
- 'only' => pht(
+ 'only' => pht(
'%s can not be used with %s.',
'--create',
'--only'),
- 'preview' => pht(
- '%s can not be used with %s.',
- '--create',
- '--preview'),
'update' => pht(
'%s can not be used with %s.',
'--create',
@@ -194,6 +191,18 @@
'param' => 'revision_id',
'help' => pht('Always update a specific revision.'),
),
+ 'draft' => array(
+ 'help' => pht(
+ 'Create a draft revision so you can look over your changes before '.
+ 'involving anyone else. Other users will not be notified about the '.
+ 'revision until you later use "Request Review" to publish it. You '.
+ 'can still share the draft by giving someone the link.'),
+ 'conflicts' => array(
+ 'edit' => null,
+ 'only' => null,
+ 'update' => null,
+ ),
+ ),
'nounit' => array(
'help' => pht('Do not run unit tests.'),
),
@@ -207,38 +216,12 @@
),
),
'only' => array(
- 'help' => pht(
- 'Only generate a diff, without running lint, unit tests, or other '.
- 'auxiliary steps. See also %s.',
- '--preview'),
- 'conflicts' => array(
- 'preview' => null,
- 'message' => pht('%s does not affect revisions.', '--only'),
- 'edit' => pht('%s does not affect revisions.', '--only'),
- 'lintall' => pht('%s suppresses lint.', '--only'),
- 'advice' => pht('%s suppresses lint.', '--only'),
- 'apply-patches' => pht('%s suppresses lint.', '--only'),
- 'never-apply-patches' => pht('%s suppresses lint.', '--only'),
- ),
- ),
- 'preview' => array(
'help' => pht(
'Instead of creating or updating a revision, only create a diff, '.
- 'which you may later attach to a revision. This still runs lint '.
- 'unit tests. See also %s.',
- '--only'),
+ 'which you may later attach to a revision.'),
'conflicts' => array(
- 'only' => null,
- 'edit' => pht('%s does affect revisions.', '--preview'),
- 'message' => pht('%s does not update any revision.', '--preview'),
- ),
- ),
- 'plan-changes' => array(
- 'help' => pht(
- 'Create or update a revision without requesting a code review.'),
- 'conflicts' => array(
- 'only' => pht('%s does not affect revisions.', '--only'),
- 'preview' => pht('%s does not affect revisions.', '--preview'),
+ 'edit' => pht('%s does affect revisions.', '--only'),
+ 'message' => pht('%s does not update any revision.', '--only'),
),
),
'encoding' => array(
@@ -351,8 +334,7 @@
'conflicts' => array(
'use-commit-message' => true,
'update' => true,
- 'only' => true,
- 'preview' => true,
+ 'only' => true,
'raw' => true,
'raw-command' => true,
'message-file' => true,
@@ -362,8 +344,7 @@
'param' => 'usernames',
'help' => pht('When creating a revision, add reviewers.'),
'conflicts' => array(
- 'only' => true,
- 'preview' => true,
+ 'only' => true,
'update' => true,
),
),
@@ -371,8 +352,7 @@
'param' => 'usernames',
'help' => pht('When creating a revision, add CCs.'),
'conflicts' => array(
- 'only' => true,
- 'preview' => true,
+ 'only' => true,
'update' => true,
),
),
@@ -393,20 +373,6 @@
),
'supports' => array('git', 'hg'),
),
- 'no-diff' => array(
- 'help' => pht(
- 'Only run lint and unit tests. Intended for internal use.'),
- ),
- 'cache' => array(
- 'param' => 'bool',
- 'help' => pht(
- '%d to disable lint cache, %d to enable (default).',
- 0,
- 1),
- 'passthru' => array(
- 'lint' => true,
- ),
- ),
'coverage' => array(
'help' => pht('Always enable coverage information.'),
'conflicts' => array(
@@ -456,14 +422,6 @@
$this->console = PhutilConsole::getConsole();
$this->runRepositoryAPISetup();
-
- if ($this->getArgument('no-diff')) {
- $this->removeScratchFile('diff-result.json');
- $data = $this->runLintUnit();
- $this->writeScratchJSONFile('diff-result.json', $data);
- return 0;
- }
-
$this->runDiffSetupBasics();
$commit_message = $this->buildCommitMessage();
@@ -566,9 +524,34 @@
$this->openURIsInBrowser(array($diff_info['uri']));
}
} else {
+ $is_draft = $this->getArgument('draft');
$revision['diffid'] = $this->getDiffID();
if ($commit_message->getRevisionID()) {
+ if ($is_draft) {
+ // TODO: In at least some cases, we could raise this earlier in the
+ // workflow to save users some time before the workflow aborts.
+ if ($this->revisionIsDraft) {
+ $this->writeWarn(
+ pht('ALREADY A DRAFT'),
+ pht(
+ 'You are updating a revision ("%s") with the "--draft" flag, '.
+ 'but this revision is already a draft. You only need to '.
+ 'provide the "--draft" flag when creating a revision. Draft '.
+ 'revisions are not published until you explicitly request '.
+ 'review from the web UI.',
+ $commit_message->getRevisionMonogram()));
+ } else {
+ throw new ArcanistUsageException(
+ pht(
+ 'You are updating a revision ("%s") with the "--draft" flag, '.
+ 'but this revision has already been published for review. '.
+ 'You can not turn a revision back into a draft once it has '.
+ 'been published.',
+ $commit_message->getRevisionMonogram()));
+ }
+ }
+
$result = $conduit->callMethodSynchronous(
'differential.updaterevision',
$revision);
@@ -579,18 +562,70 @@
$this->writeScratchJSONFile($file, $messages);
}
+ $result_uri = $result['uri'];
+ $result_id = $result['revisionid'];
+
echo pht('Updated an existing Differential revision:')."\n";
} else {
- $revision = $this->dispatchWillCreateRevisionEvent($revision);
+ // NOTE: We're either using "differential.revision.edit" (preferred)
+ // if we can, or falling back to "differential.createrevision"
+ // (the older way) if not.
+
+ $xactions = $this->revisionTransactions;
+ if ($xactions) {
+ $xactions[] = array(
+ 'type' => 'update',
+ 'value' => $diff_info['phid'],
+ );
+
+ if ($is_draft) {
+ $xactions[] = array(
+ 'type' => 'draft',
+ 'value' => true,
+ );
+ }
- $result = $conduit->callMethodSynchronous(
- 'differential.createrevision',
- $revision);
+ $result = $conduit->callMethodSynchronous(
+ 'differential.revision.edit',
+ array(
+ 'transactions' => $xactions,
+ ));
+
+ $result_id = idxv($result, array('object', 'id'));
+ if (!$result_id) {
+ throw new Exception(
+ pht(
+ 'Expected a revision ID to be returned by '.
+ '"differential.revision.edit".'));
+ }
+
+ // TODO: This is hacky, but we don't currently receive a URI back
+ // from "differential.revision.edit".
+ $result_uri = id(new PhutilURI($this->getConduitURI()))
+ ->setPath('/D'.$result_id);
+ } else {
+ if ($is_draft) {
+ throw new ArcanistUsageException(
+ pht(
+ 'You have specified "--draft", but the version of Phabricator '.
+ 'on the server is too old to support draft revisions. Omit '.
+ 'the flag or upgrade the server software.'));
+ }
+
+ $revision = $this->dispatchWillCreateRevisionEvent($revision);
+
+ $result = $conduit->callMethodSynchronous(
+ 'differential.createrevision',
+ $revision);
+
+ $result_uri = $result['uri'];
+ $result_id = $result['revisionid'];
+ }
$revised_message = $conduit->callMethodSynchronous(
'differential.getcommitmessage',
array(
- 'revision_id' => $result['revisionid'],
+ 'revision_id' => $result_id,
));
if ($this->shouldAmend()) {
@@ -608,22 +643,12 @@
echo pht('Created a new Differential revision:')."\n";
}
- $uri = $result['uri'];
+ $uri = $result_uri;
echo phutil_console_format(
" **%s** __%s__\n\n",
pht('Revision URI:'),
$uri);
- if ($this->getArgument('plan-changes')) {
- $conduit->callMethodSynchronous(
- 'differential.createcomment',
- array(
- 'revision_id' => $result['revisionid'],
- 'action' => 'rethink',
- ));
- echo pht('Planned changes to the revision.')."\n";
- }
-
if ($this->shouldOpenCreatedObjectsInBrowser()) {
$this->openURIsInBrowser(array($uri));
}
@@ -706,6 +731,7 @@
$revision = array(
'fields' => $message->getFields(),
);
+ $xactions = $message->getTransactions();
if ($revision_id) {
@@ -760,6 +786,7 @@
}
$revision['fields'] = $new_message->getFields();
+ $xactions = $new_message->getTransactions();
$revision['id'] = $revision_id;
$this->revisionID = $revision_id;
@@ -782,6 +809,8 @@
}
}
+ $this->revisionTransactions = $xactions;
+
return $revision;
}
@@ -802,8 +831,7 @@
return true;
}
- return $this->getArgument('preview') ||
- $this->getArgument('only');
+ return $this->getArgument('only');
}
private function generateAffectedPaths() {
@@ -1224,7 +1252,6 @@
*/
private function runLint() {
if ($this->getArgument('nolint') ||
- $this->getArgument('only') ||
$this->isRawDiffSource() ||
$this->getArgument('head')) {
return ArcanistLintWorkflow::RESULT_SKIP;
@@ -1305,7 +1332,6 @@
*/
private function runUnit() {
if ($this->getArgument('nounit') ||
- $this->getArgument('only') ||
$this->isRawDiffSource() ||
$this->getArgument('head')) {
return ArcanistUnitWorkflow::RESULT_SKIP;
@@ -1449,7 +1475,7 @@
* @task message
*/
private function buildCommitMessage() {
- if ($this->getArgument('preview') || $this->getArgument('only')) {
+ if ($this->getArgument('only')) {
return null;
}
@@ -1767,6 +1793,12 @@
$this->checkRevisionOwnership($revision);
+ // TODO: Save this status to improve a prompt later. See PHI458. This is
+ // extra awful until we move to "differential.revision.search" because
+ // the "differential.query" method doesn't return a real draft status for
+ // compatibility.
+ $this->revisionIsDraft = (idx($revision, 'statusName') === 'Draft');
+
$message = $this->getConduit()->callMethodSynchronous(
'differential.getcommitmessage',
array(
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mar 22 2025, 3:54 AM (4 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7716390
Default Alt Text
D20985.id50003.diff (13 KB)
Attached To
Mode
D20985: Merge "--draft" flag and related changes from "experimental" to "master"
Attached
Detach File
Event Timeline
Log In to Comment