Summary: Fixes T903. Knock out the side nav, make it policy-aware, other minor cleanup.
Test Plan: See below.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T903
Differential Revision: https://secure.phabricator.com/D7064
Summary:
- Kicks it out to full width.
- More useful header/crumbs/properties/actions (needs some more work).
- Works for public repositories.
- Fix a bug where the "rX" crumb would lose the branch you're on.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7063
Summary: Get rid of remaining callsites for buildStandardPageResponse() and modernize the UIs.
Test Plan: Looked at branches, tags, and commit detail pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7062
Summary: Ref T603. Allows permitted users to set view and edit policies for repositories. So far the repository list, repository detail, repository edit, and browse interfaces respect these settings. Most other interfaces will respect stricter settings, but "Public" won't work. Lots of rough edges in the integration still. None of this makes policies any looser than they were already without explicit user intervention, so I just put a warning about it in the UI.
Test Plan: Set a repository to public and browsed it. Verified I could not access non-public repositories.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, davidressman
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7061
Summary: We currently render something kind of goofy; integrate these with the other actions.
Test Plan: Viewed `aphlict.swf`, some PNG in Diffusion.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7052
Summary:
We have this silly "view" preference which has a variety of silly values: "plain", "plainblame", "highlighted", and "blame", and then also "raw", which is magical. This is really just two flags: color on/off, and blame on/off (plus a separate mode for raw).
Express the code in terms of the flags and, e.g., get rid of the state transition tables we had before.
Test Plan: Viewed code in all four modes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7046
Summary: This needs some more cleanup, but gets us a step closer to something reasonable.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7043
Summary: Broadly, I'm trying to modernize these views and fix UI and at least mitigate mobile problems. See discussion.
Test Plan: See screenshots.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7042
Summary: Get thee modernized.
Test Plan: See screenshot.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7040
Summary: Fixes T3840. Depends on D7021. See task for discussion. Also improved some config/help stuff.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3840
Differential Revision: https://secure.phabricator.com/D7022
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
Summary: Simplify rendering of the repository list. For inactive repositories, mark them disabled.
Test Plan: {F57615}
Reviewers: btrahan, rockybean
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6921
Summary:
These need to die soon since they're not structurally policy-aware, but keep them around for the moment until we can replace them.
There is no UI to create these, and only Facebook has them.
Test Plan: {F57614}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6920
Summary:
Ref T2625. Switches Diffusion to ApplicationSearch. Notes:
- Rendering is a bit rough, I'll clean that up next.
- Ordering is a bit arbitrary, also coming shortly.
Test Plan: Used `/diffusion/` to execute various searches.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6917
Summary: We should bring these back some day, but they should be denormalized, inside the query, and there should be a better pipeline to build them in the first place. Just get rid of them for now; this essentially impacts only us.
Test Plan: Loaded `/diffusion/`, same page minus lint counts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, vrana
Differential Revision: https://secure.phabricator.com/D6915
Summary: Ref T2625. `DiffusionHomeController` currently runs these queries inline. Move them into `DiffusionRepositoryQuery`. Prepareds for ApplicationSearch.
Test Plan: Loaded `/diffusion/`, saw the same content as before.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6914
Summary: Adds plain support for object lists that just look like lists
Test Plan: review UIexamples and a number of other applications
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6922
Summary: See IRC. We currently render a "show all changes" button for commits which have more than 100 but fewer than 1000 changes, but it doesn't actually do anything. Make it do what it's supposed to.
Test Plan: Set the limit to 2; clicked the button.
Reviewers: chad, staticshock, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6900
Summary: When I swapped the views, I accidentally removed some controller -> view -> controller logic which is used to figure out which packages are highlighted. This code is a mess, but fix the feature for now and we can clean it up later.
Test Plan: {F56335}
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6835
Summary: Some more callsites, let me know if you see others, I think think is 98% of them now.
Test Plan: tested each page
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6814
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.
TODO: will take another pass as consolidating these colors and new gradients in another diff.
Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D6806
Summary:
^\s+(['"])dust\1\s*=>\s*true,?\s*$\n
Test Plan: Looked through the diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6769
Summary: Fixes T3697. Currently, we don't pass "branch" implicitly, so, e.g., when viewing a branch you don't get the right commit hash when looking up the README.
Test Plan: Viewed a non-`master` branch with a README, no fatal. Poked around and couldn't find anything suspicious.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3697
Differential Revision: https://secure.phabricator.com/D6734
Summary:
Add the ability to select singular and multiple lines in paste to highlight.
This is related to T3627
Test Plan: Create a paste, select one or more lines.
Reviewers: epriestley, tberman
Reviewed By: epriestley
CC: aran, chad
Maniphest Tasks: T3627
Differential Revision: https://secure.phabricator.com/D6668
Summary:
Ref T3373. The submit listener doesn't properly scope the form it listens to right now, so several forms on the page mean that comments post to one of them more or less at random.
Scope it properly by telling it which object PHID it is associated with.
Test Plan: Made Question comments, saw comments Ajax in on the question itself rather than on an arbitrary answer.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3373
Differential Revision: https://secure.phabricator.com/D6611
Summary: Replace giant table with PHUIStatusListView. Also remove "MetaMTA Transcripts" (which doesn't work any more) and "Herald Transcripts" (which no one uses).
Test Plan:
{F51437}
{F51438}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6562
Summary: Ref T2231. Ref T2232. This form doesn't do anything yet and there are no link sto it, but it lets you enter all the data to create a repository in a relatively simple, straightforward way.
Test Plan:
{F49740}
{F49741}
{F49742}
{F49743}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231, T2232
Differential Revision: https://secure.phabricator.com/D6430
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.
Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.
Test Plan: played around with a mock all logged out (Ref T2652) and it worked!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2691
Differential Revision: https://secure.phabricator.com/D6416
Summary: Fixes spacing in Differential revision detail and Diffusion browse views.
Test Plan:
{F48677}
{F48678}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6359
Summary: Ref T603. This query isn't policy-aware yet, but prepare for it to be one day.
Test Plan: Looked at: home page; differential home; differential detail; diffusion browse. Made differential.query conduit call.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D6337
Summary: We were missing this option on the response, so notifications about commits (e.g., tokens given) don't clear when viewing the commit.
Test Plan: Gave a commit a token with user A, viewed notification with user B, viewed commit with user B and verified notification cleared.
Reviewers: garoevans, btrahan
Reviewed By: garoevans
CC: aran
Differential Revision: https://secure.phabricator.com/D6041
Summary: Adds support for the "encoding" field to the new transactional interface.
Test Plan:
{F44189}
{F44190}
Some of the encodings in the second screen are from testing, and can no longer be set.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6035
Summary:
At one point this was sort of a one-line summary but it isn't really anymore (and doesn't appear on the list view). We could add a summary in the future if we wanted.
- Change the control from a text area to a remarkup area.
- Change the display to remarkup.
Test Plan:
{F44183}
{F44184}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6034
Summary:
Ref T2231, T603. Plan of attack here is pretty much:
- Built out a new (currently not linked in the UI) edit interface in Diffusion which is transaction-based and has a sensible layout.
- Build out a new create interface based on PagedForm which dumps into the new edit interface.
- Throw the old stuff away.
- Everyone lives happily ever after.
Test Plan:
{F44163}
{F44164}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D6029
Summary:
Ref T2784. Relatively complicated one as this bad boy is used in a repository daemon.
While testing, I noticed bugs in the expandshortname query stuff. Those variables are private to the parent class so they need some setX love.
Also, was unable to find links to the "before" stuff, but made them by hand by looking at some of these T2784 diffs, browsing a file at a specific revision, then hacking the "before" variable to be some known commit that also touched the file. This produced sensical results. On the process of doing that I upgraded a query to use the proper policy query.
Test Plan: In git, mercurial, svn, verified on a commit page the "parents" showed up correctly. played around with ?before parameter on specific file browse page, with commits known to have interesting history and stuff looked good
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5988
Summary: Ref T2784. Also sneaks in a fix for branch query -- forgot to catch the unsupported VCS exception. One strange thing is I have test Phabricator repositories and the mercurial one is showing different data than git for the same commit. The data shown is consistent pre and post this diff though so its an existing issue. Also note the mercurial is an import of git so maybe its busted-ish?
Test Plan: viewed commits in mercurial, svn, and git that were merge commits. saw the right stuff in mercurial and git.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5985
Summary: Ref T2784
Test Plan: for each flavor of VCS, I loaded up the repository home page. verified I saw some parent action where appropos. next, clicked through to 'view history' and verified it loaded up A-OK.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5960
Summary: Adds mobile support to Audit, converts tables to object item views. I also colored 'concerns' and 'audit required' in the list, but nothing else. We can add more if needed but I'm assuming these are the two most important cases.
Test Plan: Tested as much as I could, a little unsure of a few things since my local repo isn't super filled. Will let epriestley run through.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5962
Summary: Ref T2784.
Test Plan: loaded up a git commit with refs and they showed up! loaded up a git commit without revs and nothing showed up.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5957
Summary: Ref T2784.
Test Plan: loaded up my git and mercurial copies of Phabricator. Searched for "diff". Observed many results and pagination working correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5955
Summary: Ref T2784. This one was a wee bit complicated. Had to add PhabricatorUser and concept of initFromConduit (or not) to DiffusionRequest.
Test Plan: foreach repo, visited CALLSIGN and clicked a commit and verified they laoded correctly. Hacked code to hit NOT via Conduit and repeated tests to great success.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5928
Summary: title does not say it all; also added conduit wrappers for rawdiffquery and lastmodifiedquery. These get the wrapper treatment since they are used in daemons. Ref T2784.
Test Plan: for each of the 3 VCS, did the following: 1) loaded up CALLSIGN and verified 'Modified' column data showed up correctly in Browse Repository box. 2) loaded up the "change" view for a specific file and verified content showed up correctly. 3) loaded up a specific commit and noted the changes ajax loaded A-OK
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5896
Summary:
`number_format()` returns a string, so if passed a number greater than 999 by default it will add commas, which parsed by %d will only return the 1000's delimter.
```echo sprintf("%d", number_format(1000)); // Outputs 1
echo sprintf("%s", number_format(1000)); // Outputs 1,000
Test Plan: Looked at repos with over 999 commits.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5913
Summary: Converts to ObjectList for display, pht's most everything, some responsive design when possible. Tables still are tables, but scroll on touch.
Test Plan: Use iOS simulator on Diffusion, Chrome
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5847
Summary: title. Ref T2784.
Test Plan: foreach of SVN, Mercurial, and Git, loaded up a repository. Verified that only git had a tags box and it showed up correctly. Went to CALLSIGN/tags and verified that only git had a tags box and it showed up correctly. Went to various commits across vcs and verified it said "none" unless it was a git commit that also was tagged.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5894
Summary: see title. Ref T2784.
Test Plan:
In diffusion, for each of SVN, Mercurial, and Git, I loaded up /diffusion/CALLSIGN/. I verified the README was displayed and things looked good. Next I clicked on "browse" on the top-most commit and verified things looked correct. Also clicked through to a file for a good measure and things looked good.
In owners, for each of SVN, Mercurial, and Git, I played around with the path typeahead / validator. It worked correctly!
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5883
Summary: Ref T2784. This is probably pretty good except the fancy lint error saver now issue serial queries via Conduit.
Test Plan: reparsed commits on 3 repos - yay. viewed readme from diffusion UI on 3 repos - yay. viewed file content from diffusion UI on 3 repos - yay.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5824
Summary: ref T2784. This one had a few fun spots where I had to move data around. Also, is there some common object (or should I add it?) that can do this toDictionary newFromConduit stuff? Also, this assumes D5803 is largely correct at the time of this diff.
Test Plan: browsed mercurial and git repository page. saw the branches i expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5810
Summary: le title. However, this also "is the first" conversion so sets the precedence for how this will all work. See comments in the code. I think this helps us keep the new code we're writing to a minimum. Wondering if the conduit end point could be more generic, and rather than have a switch statement on VCS type, one can just implement the "handleSubversion" version and have that called? Ref T2784
Test Plan: slapped an "or true" in the conditional protecting this code path. verified it worked on all 3 vcs systems, including typing in garbage and getting a 404
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2784
Differential Revision: https://secure.phabricator.com/D5803
Summary: We may later integrate it in the global search but I want to leave it here too for the case that you want to search just some repository or some part of it.
Test Plan: Browsed repo in SVN and Git, searched in Git.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Differential Revision: https://secure.phabricator.com/D5738
Summary: Fixes T2971.
Test Plan:
/rG1.
Set regexp to one line and URL, then /rG1.
Set regexp to two lines, then /rG1.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2971
Differential Revision: https://secure.phabricator.com/D5676
Summary:
I've hit this error by exhausting memory limit on blaming a big file with lots of unknown authors.
It triggered the error ~1000 times with stack trace containing the whole ~100 kB file.
The memory ran out when it tried to JSON serialize the stack traces for the DarkConsole.
Test Plan: Blamed file with unknown authors.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5730
Summary:
The original code seemed to assume the last level of edges was the destination edge, when it was an array under the destination edge key.
Using `array_keys` fixes the crashes that resulted when commits had tasks, projects or revisions attached.
Test Plan: Open up a commit with a revision attached.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, vrana
Differential Revision: https://secure.phabricator.com/D5698
Summary: Migration doesn't delete differential.revisionPHID but maybe it should?
Test Plan: Reparsed commit, ran the migration, deleted differential.revisionPHID, looked at task with attached commit with attached revision.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5634
Summary:
Also join concepts of installed and enabled applications.
Also respect uninstalled Maniphest where disabled Maniphest was checked.
Test Plan:
Visited T1, D1.
Uninstalled Maniphest then visited T1, D1.
Disabled Maniphest then visited T1.
Visited /config/edit/maniphest.enabled/.
Reviewers: epriestley, Afaque_Hussain, edward
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5602
Summary:
We are loading blame on background which confuses me a bit - I'm not sure if it's still loading or it's already loaded and the commit changed lots of lines (so I don't see the author) and it was a long time ago (so I don't see green).
Provide a visual feedback even for very old commits.
I want to point out that I really enjoy this kind of work.
Also, this diff is my masterpiece at least for today.
Test Plan: ?view=blame - verified that the gray changed to a decent shade of green even for very old commits.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5292
Test Plan:
Applied the patch.
Looked at blame and plain blame of SVN and Git file.
Ran the lint saver.
Looked at lint messages list.
/diffusion/lint/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5218
Summary:
I have blame enabled by default and displaying files with long history takes easily over 10 seconds.
Load the blame data by AJAX instead.
This is actually doing more work and the total response time is longer but it's worth it for me as I am interested just in the file contents quite often.
I know you were talking about building blame cache but until we have it...
I'm not sure if the AJAX loading indicator in bottom right corner is enough to inform the user that we are loading it on background.
Test Plan:
?view=highlighted
?view=plainblame
?view=blame
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5244
Summary:
Provide a viewer to all remarkup engines.
This fixes commit summaries in Diffusion, which were failing to link because they didn't have a user and thus couldn't see/load `D123`, e.g.
Test Plan: Grepped for engine creation.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5152
Summary:
Unmuck almost all of the we-sort-of-have-viewers-some-of-the-time mess.
There are a few notable cases here:
- I used Omnipotent users when indexing objects for search. I think this is correct; we do policy filtering when showing results.
- I cheated in a bad way in the Remarkup object rule, but fixing this requires fixing all the PhabricatorRemarkupEngine callsites (there are 85). I'll do that in the next diff.
- I cheated in a few random places, like when sending mail about package edits. These aren't a big deal.
Test Plan:
- Grepped for all PhabricatorObjectHandleData references.
- Gave them viewers.
Reviewers: vrana
Reviewed By: vrana
CC: aran, edward
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D5151
Summary: Latest commit in repositories are displayed together with their summaries if available
Test Plan:
Diffusion did not crash - a good sign
commit summaries also only appear on Diffusion Home //only// - as expected
Reviewers: epriestley, btrahan, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5117
Summary:
Fixes T2563. Instead of rendering "rPnnnnnn", render "rPnnnnnn: add feature X". Tweak Audit tables to accommodate.
@vrana / @nh, this migration might take a while. You could safely skip it when deploying and then run it after deployment.
I think I fixed all the other places where these render, but might have missed something.
Test Plan:
- Ran first schema migration, clicked around to make sure nothing broke.
- Ran `scripts/repository/reparse.php --message rXyyyyy`, verified summary populated.
- Ran second migration.
- Checked task/diffusion/audit/differential for weird rendering.
Reviewers: vrana
Reviewed By: vrana
CC: nh, aran, chrisbolt, allixsenos
Maniphest Tasks: T2563
Differential Revision: https://secure.phabricator.com/D5012
Summary: I need to go to full browse now and then to see pending revisions or lint view.
Test Plan: Clicked it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5051
Summary: Added ttl field to files. Gabage collect files with expired ttl
Test Plan: created file with a ttl. Let garbage collector run
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4987
Summary: Mostly applies a new call spacing rule; also a few things that have slipped through via pull requests and such
Test Plan: `find src/ -type f -name '*.php' | xargs -n16 arc lint --output summary --apply-patches`
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5002
Summary: I'm too lazy to attaching them for diffs where they were introduced.
Test Plan:
/
/D1, wrote comment with code snippet
DarkConsole
commit detail, wrote comment
task detail, wrote comment
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4911
Summary:
I like this abstraction better.
Result of `phutil_implode_html()` may be also used as a param of `hsprintf()`.
Test Plan: None.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4904
Summary:
Lots of killed `phutil_escape_html()`.
Done by searching for `AphrontTableView` and then `$rows` (usually) backwards.
Test Plan:
Looked at homepage.
echo id(new AphrontTableView(array(array('<'))))->render();
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4884
Summary: Done by searching for `AphrontErrorView` and then `appendChild()`.
Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4843
Summary:
See D4812.
- This preference disables the file tree completely.
- It defaults off, so users who want it will have to go turn it on.
- Maybe slightly cleaner would be doing this if the tree was hidden and then ajaxing it in if you press "F", but that's complicated and I don't want to bother.
- Generally, I think this element is useful to something like 5% of users and not useful to 95%.
Test Plan: Enabled and disabled file tree. Looked at commits and revisions; verified they reflected the setting correctly.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4813
Summary:
We are doing a better job in 1) of D3069#3 and 2) is just dumb.
Let's see if someone notices this change.
Test Plan: /settings/panel/display/
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4789
Test Plan:
Looked at file with lint errors in Diffusion.
I've also tried inline comments in Differential but it failed.
I'll try it again after you land all your diffs.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4755
Summary: It's a little bit confusing that you couldn't use %d but kind of expected taken that the number will be formatted so it's not a number anymore.
Test Plan: /paste/
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4764
Summary: @nh, does this do something reasonable on merges? We can refine the behavior ('click to show all 92 million files'), but I want to make sure it's at least feasible before we pursue it.
Test Plan: Set 1000 to "3" and looked at a change which touched 6 files.
Reviewers: nh, vrana, zjwsoft
Reviewed By: nh
CC: aran
Differential Revision: https://secure.phabricator.com/D4730
Summary: I'm going to stop showing changes for commits which touch 30,000 files, but still want to show the comment panel.
Test Plan: Looked at commits, saw comments. Mashed "Z"; haunted mode worked.
Reviewers: nh, vrana
Reviewed By: nh
CC: aran
Differential Revision: https://secure.phabricator.com/D4729
Summary: Converts various callsites from render_tag variants to tag variants.
Test Plan: See inlines.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T2432
Differential Revision: https://secure.phabricator.com/D4689
Summary: Proper fix is to do some layout work in Diffusion. Short of that, make this escape properly.
Test Plan: Viewed various crumbs, no more overescaping for non-diffusion crumbs.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4641
Summary:
- Implements `javelin_tag()`, which is `javelin_render_tag()` on top of `phutil_tag()` instead of `phutil_render_tag()`.
- Manually converts all or almost all of the trivial callsites.
Test Plan:
- Site does not seem any more broken than before.
Reviewers: vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D4639
Summary:
Created with spatch:
lang=diff
- phutil_render_tag
+ phutil_tag
(X, Y, '...')
Then searched for `&` and `<` in the output and replaced them.
Test Plan: Loaded homepage.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4503
Summary: Fixes T2243. We recently added the FileTreeView to Diffusion commits. However, if the page doesn't have any changesets (e.g., it has an error message instead, like "this commit hasn't imported yet"), we fail to build a file tree. In this case, don't try to build one.
Test Plan: Looked at not-imported and imported commits in Diffusion, saw proper rendering/crumbs and no exceptions.
Reviewers: btrahan, chad, vrana
Reviewed By: chad
CC: aran
Maniphest Tasks: T2243
Differential Revision: https://secure.phabricator.com/D4562
Summary: Fixes T2339
Test Plan: Close Audit button does not appear if audit.can-author-close-audit option is disabled
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2339
Differential Revision: https://secure.phabricator.com/D4525
Summary: Ref T2298. This seems like the least complicated reasonable order to implement.
Test Plan: Looked at repositories, saw them ordered by name.
Reviewers: vrana, btrahan, brennantaylor
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2298
Differential Revision: https://secure.phabricator.com/D4395
Summary:
Log of some FB paths takes over 10 seconds.
We query two logs only to get accurate message about lint info which is not that important.
Test Plan: Displayed and clicked on it.
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4429
Summary: Still working through basic re-design. Adds the ability to re-use panel view without the background.
Test Plan: Viewed Diffusion and Differential in Chrome, FF, Safari.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4430
Summary:
The search indexing API has several problems right now:
- Always runs in-process.
- It would be nice to push this into the task queue for performance. However, the API currently passses an object all the way through (and some indexers depend on preloaded object attributes), so it can't be dumped into the task queue at any stage since we can't serialize it.
- Being able to use the task queue will also make rebuilding indexes faster.
- Instead, make the API phid-oriented.
- No uniform indexing API.
- Each "Editor" currently calls SomeCustomIndexer::indexThing(). This won't work with AbstractTransactions. The API is also just weird.
- Instead, provide a uniform API.
- No uniform CLI.
- We have `scripts/search/reindex_everything.php`, but it doesn't actually index everything. Each new document type needs to be separately added to it, leading to stuff like D3839. Third-party applications can't provide indexers.
- Instead, let indexers expose documents for indexing.
- Not application-oriented.
- All the indexers live in search/ right now, which isn't the right organization in an application-orietned view of the world.
- Instead, move indexers to applications and load them with SymbolLoader.
Test Plan:
- `bin/search index`
- Indexed one revision, one task.
- Indexed `--type TASK`, `--type DREV`, etc., for all types.
- Indexed `--all`.
- Added the word "saboteur" to a revision, task, wiki page, and question and then searched for it.
- Creating users is a pain; searched for a user after indexing.
- Creating commits is a pain; searched for a commit after indexing.
- Mocks aren't currently loadable in the result view, so their indexing is moot.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: 20after4, aran
Maniphest Tasks: T1991, T2104
Differential Revision: https://secure.phabricator.com/D4261
Summary: most awesome is that differential-primary-pane no longer has a place in diffusion. less awesome is fixing the zebra striping on differential "Local Commits" view and making the font size of one of the table headers match the others.
Test Plan: looks good!
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D4177
Summary: we don't always have a diff so instead set an explicit title in the controller.
Test Plan: no more fatals. grepped carefully for every call site and tested them all
Summary:
- Remove "Diffusion" crumb (redundant with application icon)
- Put "All Repositories" in its place on the landing page.
- Render the repository crumb as "rX" instead of "X Repository".
Test Plan: Looked at various Diffusion pages.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D4173
Summary:
upgrades are CrumbsView, HeaderView, PropertyListView, and ActionListView. I had to modify CrumbsView stuff a bit to handle the "advanced" diffusion crumbs.
Quirks fixed include making file tree view show up in diffusion, the page not have extra space when the file tree is hidden, links no longer breaking once you visit files (since without the change the files always got "/" appended and thus 404'd), and a differential quirk where it read "next step:" and that colon is a no no,
Test Plan: played around in diffusion and differential
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T2048, T2178
Differential Revision: https://secure.phabricator.com/D4169
Summary:
AphrontSideNavView is an old class which required you to do a lot of work; it was obsoleted by AphrontSideNavFilterView. Remove all direct callsites so I can clean it up.
This is a precursor to letting me render a filter menu as a dropdown menu for T1960.
Test Plan:
Examined each interface for correct filter construction and selection:
- Browsed Diffusion
- Browsed Differential
- Browsed Files
- Browsed Slowvote
- Browsed Phriction
- Browsed repo edit interface
Grepped for `AphrontSideNavView`. The only remaining instances are in `AphrontSideNavView` itself and `AphrontSideNavFilterView` (which currently uses it).
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1960
Differential Revision: https://secure.phabricator.com/D4034
Summary:
I want to add search per owner, this is a prerequisity for it.
There's no link to this page yet, I didn't find a good place for it.
Test Plan: Displayed it, clicked around.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, scottmac
Maniphest Tasks: T2038
Differential Revision: https://secure.phabricator.com/D4067
Summary: make sure we only print the fancy tool tip thing if we have all the data we need. (fixes T2136). additionally, default $branches to array() to prevent errors from reset on null.
Test Plan: made a checkin for a mercurial repo with user "foo@bar" with no branch. verified name showed up in all views. also noted on commit detail view there was no more error about reset() on null for $branches.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T2136
Differential Revision: https://secure.phabricator.com/D4065
Summary:
Some users like monospaced textareas and others don't.
This introduces an option to set this as a user preference.
Test Plan: Enabled and saw monospaced textareas, disabled and saw non-monospaced textareas.
Reviewers: epriestley
CC: aran, Korvin
Maniphest Tasks: T2114
Differential Revision: https://secure.phabricator.com/D4037
Test Plan: Set `lintCommit` to 'x' and browsed a file.
Reviewers: nh, epriestley
Reviewed By: nh
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3976
Summary: Also remove some columns.
Test Plan: Looked at SVN dir in Diffusion.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, vsuba
Maniphest Tasks: T2038
Differential Revision: https://secure.phabricator.com/D3949
Test Plan:
/diffusion/ARC/lint/master/src/, clicked on count link.
/diffusion/ARC/browse/master/src/difference/?lint=XHP9, clicked on file name.
/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=, verified that all messages are displayed.
/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=XHP9.
/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=TXT3, verified that 0 messages are displayed.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D3929
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).
We are removing the headers for these reasons:
- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.
This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).
Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.
Reviewers: epriestley, davidrecordon
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2035
Differential Revision: https://secure.phabricator.com/D3886
Summary: we do this by passing the "seenOnBranches" commit data detail through the stack
Test Plan: browse in diffusion link worked for non-master checkins under git
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1949
Differential Revision: https://secure.phabricator.com/D3853
