Page MenuHomePhabricator

Only link symbols if there might be any
ClosedPublic

Authored by avivey on May 20 2015, 6:24 AM.

Details

Summary

fixes T8260. Only turn on symbol links if:

  • The repository has any configuration about symbols, or
  • There actually are symbols in the repository.
Test Plan

Look at revisions and files in various states of configurations and having symbols.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

avivey retitled this revision from to Only link symbols if there might be any.
avivey updated this object.
avivey edited the test plan for this revision. (Show Details)
avivey added a reviewer: epriestley.
src/applications/differential/controller/DifferentialRevisionViewController.php
309–317

I was somewhat surprised to learn that this doesn't do anything; Actual active code is in php DifferentialChangesetDetailView.

Without this section, symbols keep working even for code in comments.

epriestley edited edge metadata.

One stray var_dump().

src/applications/diffusion/controller/DiffusionBrowseFileController.php
279

When testing arrays for emptiness, prefer if ($x) over if (empty($x)) for consistency and simplicity.

286

Debugging var_dump().

This revision now requires changes to proceed.May 20 2015, 1:56 PM
avivey edited edge metadata.
avivey marked 2 inline comments as done.
  • s/empty($x)/$x
src/applications/diffusion/query/DiffusionSymbolQuery.php
133

Mind if I keep this empty(), to effectively cast this to boolean?

epriestley edited edge metadata.
This revision is now accepted and ready to land.May 21 2015, 3:25 PM
This revision was automatically updated to reflect the committed changes.