From ac0aa330f5972d7ca04483b243e3b9802e2b897a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 2 Apr 2013 14:06:46 -0700 Subject: [PATCH] Don't use `emailuser` in Mercurial Summary: The `emailuser` template is a relatively recent addition to Mercurial, and a few users have complained about it. It also doesn't actually do what I thought it did, e.g. in an address like this: "Abraham Lincoln" ^^^^^^^^^^^^^^^ ^^^^^^^^ (1) (2) ^^^^^^^^^^^^^^^^^^^^^^^ (3) ...I want (1), but `emailuser` means (2). Instead, extract (1) with `getDisplayName()` and (3) with `getAddress()` using PhutilEmailAddress. The implementation in Mercurial is not particularly sophisticated or magical (it just looks for "@" and "<") so we aren't really missing anything by doing this ourselves, at least today. Also fix some issues in `arc export`, which literally no one uses, but which is occasionally useful for testing (as here). Test Plan: - Ran `arc diff --only` in an `hg` repo, checked DB to see that name/email were correctly extracted. - Ran `arc export --git` in an `hg` repo, didn't get a long series of fatals. Reviewers: btrahan, DurhamGoode Reviewed By: DurhamGoode CC: aran Maniphest Tasks: T2866, T2858 Differential Revision: https://secure.phabricator.com/D5539 --- src/repository/api/ArcanistMercurialAPI.php | 10 +++++++--- src/workflow/ArcanistExportWorkflow.php | 18 +++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php index 541af26b..cec7b38f 100644 --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -199,8 +199,8 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { if ($this->localCommitInfo === null) { $base_commit = $this->getBaseCommit(); list($info) = $this->execxLocal( - "log --template '%C' --rev %s --branch %s --", - "{node}\1{rev}\1{author|emailuser}\1{author|email}\1". + "log --template %s --rev %s --branch %s --", + "{node}\1{rev}\1{author}\1". "{date|rfc822date}\1{branch}\1{tag}\1{parents}\1{desc}\2", hgsprintf('(%s::. - %s)', $base_commit, $base_commit), $this->getBranchName()); @@ -212,9 +212,13 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { $commits = array(); foreach ($logs as $log) { - list($node, $rev, $author, $author_email, $date, $branch, $tag, + list($node, $rev, $full_author, $date, $branch, $tag, $parents, $desc) = explode("\1", $log, 9); + $email = new PhutilEmailAddress($full_author); + $author = $email->getDisplayName(); + $author_email = $email->getAddress(); + // NOTE: If a commit has only one parent, {parents} returns empty. // If it has two parents, {parents} returns revs and short hashes, not // full hashes. Try to avoid making calls to "hg parents" because it's diff --git a/src/workflow/ArcanistExportWorkflow.php b/src/workflow/ArcanistExportWorkflow.php index fcf8ce09..fafde4cc 100644 --- a/src/workflow/ArcanistExportWorkflow.php +++ b/src/workflow/ArcanistExportWorkflow.php @@ -186,11 +186,14 @@ EOTEXT 'phids' => array($this->getUserPHID()), )); $author_dict = reset($authors); + + list($email) = $repository_api->execxLocal('config user.email'); + $author = sprintf('%s <%s>', $author_dict['realName'], - $repository_api->execxLocal('config user.email')); + $email); } else if ($repository_api instanceof ArcanistMercurialAPI) { - $repository_api->parseBaseCommitArgument($this->getArgument('paths')); + $this->parseBaseCommitArgument($this->getArgument('paths')); $diff = $repository_api->getFullMercurialDiff(); $changes = $parser->parseDiff($diff); $authors = $this->getConduit()->callMethodSynchronous( @@ -198,10 +201,8 @@ EOTEXT array( 'phids' => array($this->getUserPHID()), )); - $author_dict = reset($authors); - $author = sprintf('%s <%s>', - $author_dict['realName'], - $repository_api->execxLocal('showconfig ui.username')); + + list($author) = $repository_api->execxLocal('showconfig ui.username'); } else { // TODO: paths support $paths = $repository_api->getWorkingCopyStatus(); @@ -216,7 +217,10 @@ EOTEXT $bundle->setBaseRevision( $repository_api->getSourceControlBaseRevision()); // note we can't get a revision ID for SOURCE_LOCAL - $bundle->setAuthor($author); + + $parser = new PhutilEmailAddress($author); + $bundle->setAuthorName($parser->getDisplayName()); + $bundle->setAuthorEmail($parser->getAddress()); break; case self::SOURCE_REVISION: $bundle = $this->loadRevisionBundleFromConduit(