1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

Tie all the pieces for symbol cross-references together

Summary:
This makes symbol cross-references work in Differential. You need to do a little
legwork but I'll document that once the change has baked for a little while.

Basically:

  - Projects are annotated with indexed languages, and "shared library" projects
(for example, symbols in Phabricator should be searched for in Arcanist and
libphutil).
  - When we render a changeset, we check if its language is an indexed one. If
it is, we invoke the decorator Javascript.
  - The Javascript takes you to a lookup page, which either gives you a list of
matching symbols (if several match) or redirects you instantly to the
definition.

Test Plan: Clicked class and function symbols in a diff, got jumped into
sensible sorts of places in Diffusion.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 980
This commit is contained in:
epriestley 2011-10-02 16:02:56 -07:00
parent 0580772805
commit 254f606e89
16 changed files with 242 additions and 13 deletions

View file

@ -0,0 +1,4 @@
ALTER TABLE phabricator_repository.repository_arcanistproject
ADD symbolIndexLanguages LONGBLOB NOT NULL;
ALTER TABLE phabricator_repository.repository_arcanistproject
ADD symbolIndexProjects LONGBLOB NOT NULL;

View file

@ -759,7 +759,7 @@ celerity_register_resource_map(array(
),
'javelin-behavior-repository-crossreference' =>
array(
'uri' => '/res/e80365e9/rsrc/js/application/repository/repository-crossreference.js',
'uri' => '/res/49472f48/rsrc/js/application/repository/repository-crossreference.js',
'type' => 'js',
'requires' =>
array(

View file

@ -180,11 +180,11 @@ class DifferentialRevisionViewController extends DifferentialController {
$whitespace = $request->getStr(
'whitespace',
DifferentialChangesetParser::WHITESPACE_IGNORE_ALL
);
DifferentialChangesetParser::WHITESPACE_IGNORE_ALL);
$symbol_indexes = $this->buildSymbolIndexes($target, $visible_changesets);
$revision_detail->setActions($actions);
$revision_detail->setUser($user);
$comment_view = new DifferentialRevisionCommentListView();
@ -202,6 +202,7 @@ class DifferentialRevisionViewController extends DifferentialController {
$changeset_view->setRevision($revision);
$changeset_view->setRenderingReferences($rendering_references);
$changeset_view->setWhitespace($whitespace);
$changeset_view->setSymbolIndexes($symbol_indexes);
$diff_history = new DifferentialRevisionUpdateHistoryView();
$diff_history->setDiffs($diffs);
@ -571,5 +572,40 @@ class DifferentialRevisionViewController extends DifferentialController {
$special_properties));
}
private function buildSymbolIndexes(
DifferentialDiff $target,
array $visible_changesets) {
$engine = PhabricatorSyntaxHighlighter::newEngine();
$symbol_indexes = array();
$arc_project = $target->loadArcanistProject();
if (!$arc_project) {
return array();
}
$langs = $arc_project->getSymbolIndexLanguages();
if (!$langs) {
return array();
}
$project_phids = array_merge(
array($arc_project->getPHID()),
nonempty($arc_project->getSymbolIndexProjects(), array()));
$indexed_langs = array_fill_keys($langs, true);
foreach ($visible_changesets as $key => $changeset) {
$lang = $engine->getLanguageFromFilename($changeset->getFileName());
if (isset($indexed_langs[$lang])) {
$symbol_indexes[$key] = array(
'lang' => $lang,
'projects' => $project_phids,
);
}
}
return $symbol_indexes;
}
}

View file

@ -29,6 +29,7 @@ phutil_require_module('phabricator', 'applications/differential/view/revisioncom
phutil_require_module('phabricator', 'applications/differential/view/revisiondetail');
phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory');
phutil_require_module('phabricator', 'applications/draft/storage/draft');
phutil_require_module('phabricator', 'applications/markup/syntax');
phutil_require_module('phabricator', 'applications/phid/handle/data');
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'infrastructure/env');

View file

@ -21,6 +21,7 @@ class DifferentialChangesetDetailView extends AphrontView {
private $changeset;
private $buttons = array();
private $revisionID;
private $symbolIndex;
public function setChangeset($changeset) {
$this->changeset = $changeset;
@ -37,6 +38,11 @@ class DifferentialChangesetDetailView extends AphrontView {
return $this;
}
public function setSymbolIndex($symbol_index) {
$this->symbolIndex = $symbol_index;
return $this;
}
public function render() {
require_celerity_resource('differential-changeset-view-css');
require_celerity_resource('syntax-highlighting-css');
@ -61,6 +67,16 @@ class DifferentialChangesetDetailView extends AphrontView {
'</div>';
}
$id = celerity_generate_unique_node_id();
if ($this->symbolIndex) {
Javelin::initBehavior(
'repository-crossreference',
array(
'container' => $id,
) + $this->symbolIndex);
}
$display_filename = $changeset->getDisplayFilename();
$output = javelin_render_tag(
'div',
@ -71,6 +87,7 @@ class DifferentialChangesetDetailView extends AphrontView {
'right' => $this->changeset->getID(),
),
'class' => $class,
'id' => $id,
),
phutil_render_tag(
'a',

View file

@ -7,6 +7,7 @@
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'infrastructure/javelin/api');
phutil_require_module('phabricator', 'infrastructure/javelin/markup');
phutil_require_module('phabricator', 'view/base');

View file

@ -24,6 +24,7 @@ class DifferentialChangesetListView extends AphrontView {
private $renderURI = '/differential/changeset/';
private $whitespace;
private $standaloneViews;
private $symbolIndexes = array();
public function setChangesets($changesets) {
$this->changesets = $changesets;
@ -50,6 +51,11 @@ class DifferentialChangesetListView extends AphrontView {
return $this;
}
public function setSymbolIndexes(array $indexes) {
$this->symbolIndexes = $indexes;
return $this;
}
public function setRenderURI($render_uri) {
$this->renderURI = $render_uri;
return $this;
@ -100,6 +106,7 @@ class DifferentialChangesetListView extends AphrontView {
$detail = new DifferentialChangesetDetailView();
$detail->setChangeset($changeset);
$detail->addButton($detail_button);
$detail->setSymbolIndex(idx($this->symbolIndexes, $key));
$detail->appendChild(
phutil_render_tag(
'div',
@ -136,12 +143,6 @@ class DifferentialChangesetListView extends AphrontView {
));
}
Javelin::initBehavior(
'repository-crossreference',
array(
'container' => 'differential-review-stage',
));
return
'<div class="differential-review-stage" id="differential-review-stage">'.
implode("\n", $output).

View file

@ -14,6 +14,7 @@ phutil_require_module('phabricator', 'view/base');
phutil_require_module('phutil', 'markup');
phutil_require_module('phutil', 'parser/uri');
phutil_require_module('phutil', 'utils');
phutil_require_source('DifferentialChangesetListView.php');

View file

@ -29,7 +29,7 @@ class DiffusionSymbolController extends DiffusionController {
$user = $request->getUser();
$query = new DiffusionSymbolQuery();
$query->setNamePrefix($this->name);
$query->setName($this->name);
if ($request->getStr('type')) {
$query->setType($request->getStr('type'));
@ -39,14 +39,104 @@ class DiffusionSymbolController extends DiffusionController {
$query->setLanguage($request->getStr('lang'));
}
if ($request->getStr('projects')) {
$phids = $request->getStr('projects');
$phids = explode(',', $phids);
$phids = array_filter($phids);
if ($phids) {
$projects = id(new PhabricatorRepositoryArcanistProject())
->loadAllWhere(
'phid IN (%Ls)',
$phids);
$projects = mpull($projects, 'getID');
if ($projects) {
$query->setProjectIDs($projects);
}
}
}
$symbols = $query->execute();
// For PHP builtins, jump to php.net documentation.
if ($request->getBool('jump') && count($symbols) == 0) {
if ($request->getStr('lang') == 'php') {
if ($request->getStr('type') == 'function') {
if (in_array($this->name, idx(get_defined_functions(), 'internal'))) {
return id(new AphrontRedirectResponse())
->setURI('http://www.php.net/'.$this->name);
}
}
}
}
if ($symbols) {
$projects = id(new PhabricatorRepositoryArcanistProject())->loadAllWhere(
'id IN (%Ld)',
mpull($symbols, 'getArcanistProjectID', 'getID'));
} else {
$projects = array();
}
$path_map = array();
if ($symbols) {
$path_map = queryfx_all(
id(new PhabricatorRepository())->establishConnection('r'),
'SELECT * FROM %T WHERE id IN (%Ld)',
PhabricatorRepository::TABLE_PATH,
mpull($symbols, 'getPathID'));
$path_map = ipull($path_map, 'path', 'id');
}
$repo_ids = array_filter(mpull($projects, 'getRepositoryID'));
if ($repo_ids) {
$repos = id(new PhabricatorRepository())->loadAllWhere(
'id IN (%Ld)',
$repo_ids);
} else {
$repos = array();
}
$rows = array();
foreach ($symbols as $symbol) {
$project = idx($projects, $symbol->getArcanistProjectID());
if ($project) {
$project_name = $project->getName();
} else {
$project_name = '-';
}
$file = phutil_escape_html(idx($path_map, $symbol->getPathID()));
$line = phutil_escape_html($symbol->getLineNumber());
$repo = idx($repos, $project->getRepositoryID());
if ($repo) {
$href = '/diffusion/'.$repo->getCallsign().'/browse'.$file.'$'.$line;
if ($request->getBool('jump') && count($symbols) == 1) {
// If this is a clickthrough from Differential, just jump them
// straight to the target if we got a single hit.
return id(new AphrontRedirectResponse())->setURI($href);
}
$location = phutil_render_tag(
'a',
array(
'href' => $href,
),
phutil_escape_html($file.':'.$line));
} else if ($file) {
$location = phutil_escape_html($file.':'.$line);
} else {
$location = '?';
}
$rows[] = array(
phutil_escape_html($symbol->getSymbolType()),
phutil_escape_html($symbol->getSymbolName()),
phutil_escape_html($symbol->getSymbolLanguage()),
phutil_escape_html($project_name),
$location,
);
}
@ -56,13 +146,20 @@ class DiffusionSymbolController extends DiffusionController {
'Type',
'Name',
'Language',
'Project',
'File',
));
$table->setColumnClasses(
array(
'',
'pri',
'',
'',
'',
'n'
));
$table->setNoDataString(
"No matching symbol could be found in any indexed project.");
$panel = new AphrontPanelView();
$panel->setHeader('Similar Symbols');

View file

@ -6,12 +6,17 @@
phutil_require_module('phabricator', 'aphront/response/redirect');
phutil_require_module('phabricator', 'applications/diffusion/controller/base');
phutil_require_module('phabricator', 'applications/diffusion/query/symbol');
phutil_require_module('phabricator', 'applications/repository/storage/arcanistproject');
phutil_require_module('phabricator', 'applications/repository/storage/repository');
phutil_require_module('phabricator', 'storage/queryfx');
phutil_require_module('phabricator', 'view/control/table');
phutil_require_module('phabricator', 'view/layout/panel');
phutil_require_module('phutil', 'markup');
phutil_require_module('phutil', 'utils');
phutil_require_source('DiffusionSymbolController.php');

View file

@ -46,6 +46,15 @@ class PhabricatorRepositoryArcanistProjectEditController
}
if ($request->isFormPost()) {
$indexed = $request->getStr('symbolIndexLanguages');
$indexed = strtolower($indexed);
$indexed = preg_split('/[ ,]+/', $indexed);
$indexed = array_filter($indexed);
$project->setSymbolIndexLanguages($indexed);
$project->setSymbolIndexProjects($request->getArr('symbolIndexProjects'));
$repo_id = $request->getInt('repository', 0);
if (isset($repos[$repo_id])) {
$project->setRepositoryID($repo_id);
@ -56,6 +65,22 @@ class PhabricatorRepositoryArcanistProjectEditController
}
}
$langs = $project->getSymbolIndexLanguages();
if ($langs) {
$langs = implode(', ', $langs);
} else {
$langs = null;
}
if ($project->getSymbolIndexProjects()) {
$uses = id(new PhabricatorRepositoryArcanistProject())->loadAllWhere(
'phid in (%Ls)',
$project->getSymbolIndexProjects());
$uses = mpull($uses, 'getName', 'getPHID');
} else {
$uses = array();
}
$form = id(new AphrontFormView())
->setUser($user)
->appendChild(
@ -72,13 +97,25 @@ class PhabricatorRepositoryArcanistProjectEditController
->setOptions($repos)
->setName('repository')
->setValue($project->getRepositoryID()))
->appendChild(
id(new AphrontFormTextControl())
->setLabel('Indexed Languages')
->setName('symbolIndexLanguages')
->setCaption('Separate with commas, for example: <tt>php, py</tt>')
->setValue($langs))
->appendChild(
id(new AphrontFormTokenizerControl())
->setLabel('Uses Symbols From')
->setName('symbolIndexProjects')
->setDatasource('/typeahead/common/arcanistprojects/')
->setValue($uses))
->appendChild(
id(new AphrontFormSubmitControl())
->addCancelButton('/repository/')
->setValue('Save'));
$panel = new AphrontPanelView();
$panel->setWidth(AphrontPanelView::WIDTH_FORM);
$panel->setWidth(AphrontPanelView::WIDTH_WIDE);
$panel->setHeader('Edit Arcanist Project');
$panel->appendChild($form);

View file

@ -15,6 +15,8 @@ phutil_require_module('phabricator', 'view/form/base');
phutil_require_module('phabricator', 'view/form/control/select');
phutil_require_module('phabricator', 'view/form/control/static');
phutil_require_module('phabricator', 'view/form/control/submit');
phutil_require_module('phabricator', 'view/form/control/text');
phutil_require_module('phabricator', 'view/form/control/tokenizer');
phutil_require_module('phabricator', 'view/layout/panel');
phutil_require_module('phutil', 'utils');

View file

@ -23,10 +23,17 @@ class PhabricatorRepositoryArcanistProject
protected $phid;
protected $repositoryID;
protected $symbolIndexLanguages = array();
protected $symbolIndexProjects = array();
public function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
self::CONFIG_TIMESTAMPS => false,
self::CONFIG_SERIALIZATION => array(
'symbolIndexLanguages' => self::SERIALIZATION_JSON,
'symbolIndexProjects' => self::SERIALIZATION_JSON,
),
) + parent::getConfiguration();
}

View file

@ -35,6 +35,7 @@ class PhabricatorTypeaheadCommonDatasourceController
$need_repos = false;
$need_packages = false;
$need_upforgrabs = false;
$need_arcanist_projects = false;
switch ($this->type) {
case 'searchowner':
$need_users = true;
@ -60,6 +61,10 @@ class PhabricatorTypeaheadCommonDatasourceController
$need_users = true;
$need_all_users = true;
break;
case 'arcanistprojects':
$need_arcanist_projects = true;
break;
}
$data = array();
@ -150,6 +155,17 @@ class PhabricatorTypeaheadCommonDatasourceController
}
}
if ($need_arcanist_projects) {
$arcprojs = id(new PhabricatorRepositoryArcanistProject())->loadAll();
foreach ($arcprojs as $proj) {
$data[] = array(
$proj->getName(),
null,
$proj->getPHID(),
);
}
}
return id(new AphrontAjaxResponse())
->setContent($data);
}

View file

@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'applications/metamta/storage/mailinglist')
phutil_require_module('phabricator', 'applications/owners/storage/package');
phutil_require_module('phabricator', 'applications/people/storage/user');
phutil_require_module('phabricator', 'applications/project/storage/project');
phutil_require_module('phabricator', 'applications/repository/storage/arcanistproject');
phutil_require_module('phabricator', 'applications/repository/storage/repository');
phutil_require_module('phabricator', 'applications/typeahead/controller/base');

View file

@ -22,7 +22,10 @@ JX.behavior('repository-crossreference', function(config) {
if (JX.DOM.isNode(target, 'span') && (target.className in map)) {
var uri = JX.$U('/diffusion/symbol/' + target.innerHTML + '/');
uri.addQueryParams({
type : map[target.className]
type : map[target.className],
lang : config.lang,
projects : config.projects.join(','),
jump : true
});
window.open(uri);
e.kill();