From c0d151e0e91dc7965cb4c6357f5285c5fd71f925 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 1 May 2020 07:44:57 -0700 Subject: [PATCH] Add "--browse" to "arc upload" and update behavior, particularly "--json" Summary: Ref T13528. Provide a "--browse" flag to open files after they are uploaded. Update the code to use modern query strategies. This impacts the output of "--json", which was just raw "file.info" output before and could not represent the same path being passed several times (`arc upload X X`). Test Plan: Ran `arc upload` with `--browse` and `--json`. Maniphest Tasks: T13528 Differential Revision: https://secure.phabricator.com/D21202 --- .../workflow/ArcanistBrowseWorkflow.php | 15 +--- src/ref/file/ArcanistFileRef.php | 10 +++ src/workflow/ArcanistUploadWorkflow.php | 74 ++++++++++++++----- src/workflow/ArcanistWorkflow.php | 19 +++++ 4 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/browse/workflow/ArcanistBrowseWorkflow.php b/src/browse/workflow/ArcanistBrowseWorkflow.php index 85143353..ffc41654 100644 --- a/src/browse/workflow/ArcanistBrowseWorkflow.php +++ b/src/browse/workflow/ArcanistBrowseWorkflow.php @@ -234,21 +234,8 @@ EOTEXT $ref_uri = head($ref_uris); - // TODO: "ArcanistRevisionRef", at least, may return a relative URI. - // If we get a relative URI, guess the correct absolute URI based on - // the Conduit URI. This might not be correct for Conduit over SSH. - $raw_uri = $ref_uri->getURI(); - - $raw_uri = new PhutilURI($raw_uri); - if (!strlen($raw_uri->getDomain())) { - $base_uri = $this->getConduitEngine() - ->getConduitURI(); - - $raw_uri = id(new PhutilURI($base_uri)) - ->setPath($raw_uri->getPath()); - } - $raw_uri = phutil_string_cast($raw_uri); + $raw_uri = $this->getAbsoluteURI($raw_uri); $uris[] = $raw_uri; } diff --git a/src/ref/file/ArcanistFileRef.php b/src/ref/file/ArcanistFileRef.php index de588bd0..37fcf520 100644 --- a/src/ref/file/ArcanistFileRef.php +++ b/src/ref/file/ArcanistFileRef.php @@ -37,6 +37,16 @@ final class ArcanistFileRef return idxv($this->parameters, array('fields', 'size')); } + public function getURI() { + $uri = idxv($this->parameters, array('fields', 'uri')); + + if ($uri === null) { + $uri = '/'.$this->getMonogram(); + } + + return $uri; + } + public function getMonogram() { return 'F'.$this->getID(); } diff --git a/src/workflow/ArcanistUploadWorkflow.php b/src/workflow/ArcanistUploadWorkflow.php index e3506cb6..c9c9dd83 100644 --- a/src/workflow/ArcanistUploadWorkflow.php +++ b/src/workflow/ArcanistUploadWorkflow.php @@ -23,6 +23,10 @@ EOTEXT return array( $this->newWorkflowArgument('json') ->setHelp(pht('Output upload information in JSON format.')), + $this->newWorkflowArgument('browse') + ->setHelp( + pht( + 'After the upload completes, open the files in a web browser.')), $this->newWorkflowArgument('temporary') ->setHelp( pht( @@ -42,6 +46,7 @@ EOTEXT $is_temporary = $this->getArgument('temporary'); $is_json = $this->getArgument('json'); + $is_browse = $this->getArgument('browse'); $paths = $this->getArgument('paths'); $conduit = $this->getConduitEngine(); @@ -65,35 +70,68 @@ EOTEXT $files = $uploader->uploadFiles(); - $results = array(); + $phids = array(); foreach ($files as $file) { - // TODO: This could be handled more gracefully; just preserving behavior - // until we introduce `file.query` and modernize this. + // TODO: This could be handled more gracefully. if ($file->getErrors()) { throw new Exception(implode("\n", $file->getErrors())); } - $phid = $file->getPHID(); - $name = $file->getName(); + $phids[] = $file->getPHID(); + } - $info = $conduit->resolveCall( - 'file.info', - array( - 'phid' => $phid, - )); + $symbols = $this->getSymbolEngine(); + $symbol_refs = $symbols->loadFilesForSymbols($phids); - $results[$path] = $info; - - if (!$is_json) { - $id = $info['id']; - echo " F{$id} {$name}: ".$info['uri']."\n\n"; + $refs = array(); + foreach ($symbol_refs as $symbol_ref) { + $ref = $symbol_ref->getObject(); + if ($ref === null) { + throw new Exception( + pht( + 'Failed to resolve symbol ref "%s".', + $symbol_ref->getSymbol())); } + $refs[] = $ref; } if ($is_json) { - $output = id(new PhutilJSON())->encodeFormatted($results); - echo $output; + $json = array(); + + foreach ($refs as $key => $ref) { + $uri = $ref->getURI(); + $uri = $this->getAbsoluteURI($uri); + + $map = array( + 'argument' => $paths[$key], + 'id' => $ref->getID(), + 'phid' => $ref->getPHID(), + 'name' => $ref->getName(), + 'uri' => $uri, + ); + + $json[] = $map; + } + + echo id(new PhutilJSON())->encodeAsList($json); } else { - $this->writeStatus(pht('Done.')); + foreach ($refs as $ref) { + $uri = $ref->getURI(); + $uri = $this->getAbsoluteURI($uri); + echo tsprintf( + '%s', + $ref->newDisplayRef() + ->setURI($uri)); + } + } + + if ($is_browse) { + $uris = array(); + foreach ($refs as $ref) { + $uri = $ref->getURI(); + $uri = $this->getAbsoluteURI($uri); + $uris[] = $uri; + } + $this->openURIsInBrowser($uris); } return 0; diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 93e3e1ff..f4514698 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -2421,4 +2421,23 @@ abstract class ArcanistWorkflow extends Phobject { return $stdin->read(); } + protected function getAbsoluteURI($raw_uri) { + // TODO: "ArcanistRevisionRef", at least, may return a relative URI. + // If we get a relative URI, guess the correct absolute URI based on + // the Conduit URI. This might not be correct for Conduit over SSH. + + $raw_uri = new PhutilURI($raw_uri); + if (!strlen($raw_uri->getDomain())) { + $base_uri = $this->getConduitEngine() + ->getConduitURI(); + + $raw_uri = id(new PhutilURI($base_uri)) + ->setPath($raw_uri->getPath()); + } + + $raw_uri = phutil_string_cast($raw_uri); + + return $raw_uri; + } + }