Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15399080
D9489.id.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
9 KB
Referenced Files
None
Subscribers
None
D9489.id.diff
View Options
diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -16,7 +16,7 @@
*/
const GIT_MAGIC_ROOT_COMMIT = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
- private $symbolicHeadCommit = 'HEAD';
+ private $symbolicHeadCommit;
private $resolvedHeadCommit;
public static function newHookAPI($root) {
@@ -81,16 +81,18 @@
/**
* Tests if a child commit is descendant of a parent commit.
* If child and parent are the same, it returns false.
- * @param child commit SHA.
- * @param parent commit SHA.
- * @return bool
+ * @param Child commit SHA.
+ * @param Parent commit SHA.
+ * @return bool True if the child is a descendant of the parent.
*/
private function isDescendant($child, $parent) {
- list($common_ancestor) =
- $this->execxLocal('merge-base %s %s', $child, $parent);
+ list($common_ancestor) = $this->execxLocal(
+ 'merge-base %s %s',
+ $child,
+ $parent);
$common_ancestor = trim($common_ancestor);
- return $common_ancestor == $parent && $common_ancestor != $child;
+ return ($common_ancestor == $parent) && ($common_ancestor != $child);
}
public function getLocalCommitInformation() {
@@ -105,8 +107,8 @@
} else {
// 2..N commits. We include commits reachable from HEAD which are
- // not reachable from the relative commit; this is consistent with
- // user expectations even though it is not actually the diff range.
+ // not reachable from the base commit; this is consistent with user
+ // expectations even though it is not actually the diff range.
// Particularly:
//
// |
@@ -124,16 +126,38 @@
// this as being the commits X and Y. If we log "B..Y", we only show
// Y. With "Y --not B", we show X and Y.
- $base_commit = $this->getBaseCommit();
- $head_commit = $this->getHeadCommit();
- if ($this->isDescendant($head_commit, $base_commit) === false) {
- throw new ArcanistUsageException(
- "base commit ${base_commit} is not a child of head commit ".
- "${head_commit}");
+
+ if ($this->symbolicHeadCommit !== null) {
+ $base_commit = $this->getBaseCommit();
+ $resolved_base = $this->resolveCommit($base_commit);
+
+ $head_commit = $this->symbolicHeadCommit;
+ $resolved_head = $this->getHeadCommit();
+
+ if (!$this->isDescendant($resolved_head, $resolved_base)) {
+ // NOTE: Since the base commit will have been resolved as the
+ // merge-base of the specified base and the specified HEAD, we can't
+ // easily tell exactly what's wrong with the range.
+
+ // For example, `arc diff HEAD --head HEAD^^^` is invalid because it
+ // is reversed, but resolving the commit "HEAD" will compute its
+ // merge-base with "HEAD^^^", which is "HEAD^^^", so the range will
+ // appear empty.
+
+ throw new ArcanistUsageException(
+ pht(
+ 'The specified commit range is empty, backward or invalid: the '.
+ 'base (%s) is not an ancestor of the head (%s). You can not '.
+ 'diff an empty or reversed commit range.',
+ $base_commit,
+ $head_commit));
+ }
}
- $against = csprintf('%s --not %s',
- $this->getHeadCommit(), $this->getBaseCommit());
+ $against = csprintf(
+ '%s --not %s',
+ $this->getHeadCommit(),
+ $this->getBaseCommit());
}
// NOTE: Windows escaping of "%" symbols apparently is inherently broken;
@@ -197,9 +221,17 @@
"this repository.");
}
- $this->setBaseCommitExplanation(
- "it is the merge-base of '{$symbolic_commit}' and ".
- "{$this->symbolicHeadCommit}, as you explicitly specified.");
+ if ($this->symbolicHeadCommit === null) {
+ $this->setBaseCommitExplanation(
+ "it is the merge-base of the explicitly specified base commit ".
+ "'{$symbolic_commit}' and HEAD.");
+ } else {
+ $this->setBaseCommitExplanation(
+ "it is the merge-base of the explicitly specified base commit ".
+ "'{$symbolic_commit}' and the explicitly specified head ".
+ "commit '{$this->symbolicHeadCommit}'.");
+ }
+
return trim($merge_base);
}
@@ -331,8 +363,8 @@
public function getHeadCommit() {
if ($this->resolvedHeadCommit === null) {
- $this->resolvedHeadCommit =
- $this->resolveCommit($this->symbolicHeadCommit);
+ $this->resolvedHeadCommit = $this->resolveCommit(
+ coalesce($this->symbolicHeadCommit, 'HEAD'));
}
return $this->resolvedHeadCommit;
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -397,15 +397,19 @@
'*' => 'paths',
'head' => array(
'param' => 'commit',
- 'help' => "specify the head commit.\n".
- "This disables many Arcanist/Phabricator features which depend on ".
- "having access to the working copy.",
+ 'help' => pht(
+ 'Specify the end of the commit range. This disables many '.
+ 'Arcanist/Phabricator features which depend on having access to '.
+ 'the working copy.'),
'supports' => array('git'),
+ 'nosupport' => array(
+ 'svn' => pht('Subversion does not support commit ranges.'),
+ 'hg' => pht('Mercurial does not support --head yet.'),
+ ),
'conflicts' => array(
'lintall' => '--head suppresses lint.',
'advice' => '--head suppresses lint.',
),
-
)
);
@@ -447,16 +451,11 @@
$repo = $this->getRepositoryAPI();
$head_commit = $this->getArgument('head');
- $range_supported = $repo->supportsCommitRanges();
- if ($head_commit) {
- if (!$range_supported) {
- throw new ArcanistUsageException('Ranged are not supported');
- }
-
+ if ($head_commit !== null) {
$repo->setHeadCommit($head_commit);
}
- if ($range_supported) {
+ if ($repo->supportsCommitRanges()) {
$repo->getBaseCommit();
}
diff --git a/src/workflow/ArcanistWhichWorkflow.php b/src/workflow/ArcanistWhichWorkflow.php
--- a/src/workflow/ArcanistWhichWorkflow.php
+++ b/src/workflow/ArcanistWhichWorkflow.php
@@ -2,8 +2,6 @@
/**
* Show which revision or revisions are in the working copy.
- *
- * @group workflow
*/
final class ArcanistWhichWorkflow extends ArcanistBaseWorkflow {
@@ -13,8 +11,8 @@
public function getCommandSynopses() {
return phutil_console_format(<<<EOTEXT
- **which** (svn)
- **which** [commit] (hg, git)
+ **which** [options] (svn)
+ **which** [options] [__commit__] (hg, git)
EOTEXT
);
}
@@ -63,7 +61,12 @@
),
'head' => array(
'param' => 'commit',
- 'help' => 'specify the head commit.'
+ 'help' => pht('Specify the end of the commit range to select.'),
+ 'nosupport' => array(
+ 'svn' => pht('Subversion does not support commit ranges.'),
+ 'hg' => pht('Mercurial does not support --head yet.'),
+ ),
+ 'supports' => array('git'),
),
'*' => 'commit',
);
@@ -89,13 +92,9 @@
$supports_ranges = $repository_api->supportsCommitRanges();
- if ($this->getArgument('head')) {
- if ($supports_ranges === false) {
- throw new Exception('--head is not supported in this VCS');
- }
-
- $head_commit = $this->getArgument('head');
- $arg .= " --head {$head_commit}";
+ $head_commit = $this->getArgument('head');
+ if ($head_commit !== null) {
+ $arg .= csprintf(' --head %R', $head_commit);
$repository_api->setHeadCommit($head_commit);
}
@@ -128,22 +127,36 @@
if ($repository_api instanceof ArcanistGitAPI) {
$head = $this->getArgument('head', 'HEAD');
- $command = "git diff {$relative}..{$head}";
+ $command = csprintf('git diff %R', "{$relative}..{$head}");
} else if ($repository_api instanceof ArcanistMercurialAPI) {
- $command = "hg diff --rev {$relative}";
+ $command = csprintf(
+ 'hg diff --rev %R',
+ hgsprintf('%s', $relative));
} else {
throw new Exception('Unknown VCS!');
}
echo phutil_console_wrap(
phutil_console_format(
- "**RELATIVE COMMIT**\n".
+ "**COMMIT RANGE**\n".
"If you run 'arc diff{$arg}', changes between the commit:\n\n"));
echo " {$relative} {$relative_summary}\n\n";
+
+ if ($head_commit === null) {
+ $will_be_sent = pht(
+ '...and the current working copy state will be sent to '.
+ 'Differential, because %s',
+ $explanation);
+ } else {
+ $will_be_sent = pht(
+ '...and "%s" will be sent to Differential, because %s',
+ $head_commit,
+ $explanation);
+ }
+
echo phutil_console_wrap(
- "...and the current working copy state will be sent to ".
- "Differential, because {$explanation}\n\n".
+ "{$will_be_sent}\n\n".
"You can see the exact changes that will be sent by running ".
"this command:\n\n".
" $ {$command}\n\n".
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, Mar 18, 2:47 AM (1 d, 20 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7524372
Default Alt Text
D9489.id.diff (9 KB)
Attached To
Mode
D9489: Tweak error and status messages for commit ranges
Attached
Detach File
Event Timeline
Log In to Comment