1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-13 00:56:15 +01:00
Commit graph

1702 commits

Author SHA1 Message Date
epriestley
440c76eca5 Use PropertyListView in Diffusion's repository view
Summary: Precursor to adding ActionList. Ref T2231.

Test Plan: {F44153}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T2231

Differential Revision: https://secure.phabricator.com/D6027
2013-05-24 10:48:10 -07:00
Bob Trahan
824f934622 Diffusion - move commit parents query to conduit
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
2013-05-21 16:22:49 -07:00
Bob Trahan
88fec4908b Diffusion - double check these sub objects exist before toDictionary'ing them
Summary: wordy title. Ref T3214. Not exactly sure what a valid test is here as the user has a borked setup... Shouldn't fatal though.

Test Plan: arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3214

Differential Revision: https://secure.phabricator.com/D5995
2013-05-21 13:48:24 -07:00
Bob Trahan
f6b393d529 Diffusion - break out readme query all on its own
Summary: nice title. Ref T2784. Fixes T3171.

Test Plan: For all 3 VCS types, viewed phabricator repository and saw readme. browsed said repository and saw readme.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784, T3171

Differential Revision: https://secure.phabricator.com/D5993
2013-05-21 13:47:06 -07:00
Bob Trahan
5d495ebbad Diffusion - move merged commits query to happen over conduit.
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
2013-05-20 17:04:51 -07:00
epriestley
cf092781ae Fix regexp issue with Diffusion remarkup rule
Summary: My `\w` shenanigans don't work with Diffusion, which has no explicit prefix.

Test Plan: Typed `rXnnn` and got the whole thing properly marked up.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D5984
2013-05-20 13:27:03 -07:00
Bob Trahan
df50e380ca Diffusion - move history query to conduit
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
2013-05-20 12:45:34 -07:00
Joseph Perla
b09dc9cad8 Add branch as an option to recentcommitsbypath API
Summary: We need to be able to request history for more than the master branch (the default). This adds branch as an option to the API.

Test Plan: Test by sending recentcommitsbypath a non-master branch along with callsign.

Reviewers: epriestley, chad, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5967
2013-05-18 14:48:57 -07:00
Chad Little
84f21c8a20 Modernize Audit
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
2013-05-18 10:49:37 -07:00
Bob Trahan
0ade49fbe4 Diffusion - fix T3215
Summary: Ref T2784. We need to always return array() here so the foreach doesn't crap out

Test Plan: chad - can you confirm this fixes the error? I have ref-heavy git repos it seems. should work though.

Reviewers: chad, epriestley

Reviewed By: chad

CC: aran, Korvin

Maniphest Tasks: T3215, T2784

Differential Revision: https://secure.phabricator.com/D5961
2013-05-17 18:16:46 -07:00
Bob Trahan
0212d75632 Diffusion - move commit branches query to conduit
Summary: Ref T2784.

Test Plan: loaded up a mercurial and a git commit. verified branches showed up correctly in object detail panel

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784

Differential Revision: https://secure.phabricator.com/D5958
2013-05-17 16:54:10 -07:00
Bob Trahan
07ad4d154c Diffusion - move build refs to occur over Conduit
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
2013-05-17 14:13:48 -07:00
Bob Trahan
2f44c0fac9 Diffusion - move querying for stable commit name to occur over conduit
Summary: Ref T2784. This is a lower-level one from drequest so it gets the conditional initialization treatment. Consolidated SVN as well even though SVN is issuing database queries; I felt better about the code de-duplication despite the small performance hit when we could just query the DB directly in the SVN case.

Test Plan: browsed around my Phabricator repositories in Mercurial, Git, and SVN flavors. Looked good.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2784

Differential Revision: https://secure.phabricator.com/D5956
2013-05-17 14:12:46 -07:00
Bob Trahan
63e39cef32 Diffusion - move search (grep) system commands to happen over Conduit
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
2013-05-17 14:11:20 -07:00
epriestley
a548773209 Make "Receive Test" mail form use MailReceivers
Summary: Currently this is fairly hard-coded. Instead, make it use available receivers. Ref T1205.

Test Plan: Used mail form to send mail to various objects (Dnn, Tnn, Cnn, etc.). Only some of these work right now because the receiver thing still hard-codes a bunch of junk.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5944
2013-05-17 03:51:33 -07:00
Bob Trahan
016b62a7ef Diffusion - move some DiffusionRequest queries to occur over Conduit
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
2013-05-14 15:32:19 -07:00
Bob Trahan
c1d771d86c Diffusion - move DiffQuery to Conduit
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
2013-05-14 13:53:32 -07:00
Gareth Evans
2d3c73495a Allow 'readme.md' to be read as readme file in diffusion
Summary: In the title!

Test Plan: View a readme file

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5914
2013-05-13 08:25:20 -07:00
Gareth Evans
9c87ba8b09 parse with %s as number_format() returns stirng
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
2013-05-13 08:09:43 -07:00
Chad Little
57ca8de61c Modernize Diffusion
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
2013-05-11 08:23:19 -07:00
Bob Trahan
83bb113be7 Diffusion - fix error log spew from D5894
Summary: forgot to declare this method as static. PHP manages to make it work functionally anyway. Ref T2784. Fixes T3175.

Test Plan: loaded diffusion, observed no errors

Reviewers: chad, epriestley

CC: aran, Korvin

Maniphest Tasks: T3175, T2784

Differential Revision: https://secure.phabricator.com/D5899
2013-05-10 17:59:05 -07:00
Bob Trahan
c36f44a014 Diffusion - tag queries => conduit
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
2013-05-10 15:22:35 -07:00
Bob Trahan
a42851501f Diffusion - move DiffusionBrowseQuery => Conduit
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
2013-05-10 11:02:58 -07:00
deedydas
78a8079f59 Tokens added to Repository Commits and Pastes
Summary: Ref T3023

Test Plan: Tokens visible when awarded to these applications and notifications successful.

Reviewers: epriestley, AnhNhan

Reviewed By: AnhNhan

CC: aran, Korvin, AnhNhan

Maniphest Tasks: T3023

Differential Revision: https://secure.phabricator.com/D5859
2013-05-09 14:21:33 -07:00
epriestley
5877026e81 Fix highlighted code in Diffusion
Summary:
Ref T3141. This is a hacky fix until we can get something a little more sensible in place.

Currently, we sometimes end up with an empty text list, which causes file content to not display. Instead, generate the text list from the corpus if it isn't available.

Test Plan: Looked at a previously-broken source listing, saw code.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T3141

Differential Revision: https://secure.phabricator.com/D5871
2013-05-08 18:12:59 -07:00
Bob Trahan
1c8d045ea0 DiffusionFileContentQuery => Conduit
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
2013-05-07 14:57:08 -07:00
epriestley
30b15e094c Load commits affected by revert language in Diffusion message parser
Summary:
Ref T1751. This still doesn't do anything very interesting, but loads the acutal Commit objects that a commit message claims to revert.

The only tricky thing here is that we need to interpret "reverts rnnn" or "reverts nnn" in an SVN repository as "reverts rXnnn", where "X" is the current repository. This adds a method to do allow `DiffusionCommitQuery` to do that.

Test Plan:
Used `reparse.php --message` to reparse several commits with revert language and verify they loaded the correct affected commits.

In an SVN repository, created a commit with ambiguous revert language ("reverts n", "reverts rn", "reverts n, n") and verified it identified the affected commits correctly despite ambiguity.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1751

Differential Revision: https://secure.phabricator.com/D5842
2013-05-06 18:05:33 -07:00
Bob Trahan
7573ad9a70 DiffusionBranchQuery => Conduit
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
2013-05-01 14:56:36 -07:00
Bob Trahan
c79d26144a Diffusion - move DiffusionExistsQuery to work over conduit
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
2013-05-01 14:44:28 -07:00
epriestley
2ed29980db Make DiffusionQuery extend PhabricatorQuery
Summary: Ref T2683. Provides access to `formatWhereClause()`, etc. I plan to eventually unify the various DiffusionQuery execute methods as well, which all have special names right now (`loadModifiedPaths()`, `loadWhateverBlah()`, etc).

Test Plan: Ran EverythingImplemented test. Browsed Diffusion.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2683

Differential Revision: https://secure.phabricator.com/D5251
2013-04-30 10:54:02 -07:00
Jakub Vrana
f302751a23 Don't report zero grep results as error
Test Plan: Grepped for non-existing string.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5776
2013-04-24 17:10:39 -07:00
Jakub Vrana
eff3089650 Allow searching in repositories
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
2013-04-23 11:12:02 -07:00
Jakub Vrana
4767068ab7 Improve page title in Diffusion history
Test Plan: Looked at it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5748
2013-04-22 20:11:52 -07:00
Jakub Vrana
6a5807eba4 Link external bug trackers
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
2013-04-19 21:56:13 -07:00
Jakub Vrana
725373386a Pull only existing authorPHIDs in blame
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
2013-04-18 20:20:43 -07:00
Juan Pablo Civile
141228343c Fix error in D5634
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
2013-04-15 08:42:02 -07:00
Jakub Vrana
e31e998f3b Convert differential.revisionPHID commit detail to edge
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
2013-04-12 22:48:16 -07:00
epriestley
985a85ec72 Show diffs of removed and propchanged files in Mercurial correctly
Summary:
By default, `hg log -- x` does not show commits which remove the file `x`, nor commits which only change properties on `x`. By passing the flag `--removed`, commits which remove or just change properties are shown. We expect these commits to be shown in callers (this is the default behavior in Git).

Fixes T2608.

Test Plan: Created commits which remove a file and change properties on a file. Verified `hg log --removed -- x` reported them correctly, and Diffusion showed them correctly.

Reviewers: btrahan, DurhamGoode

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2608

Differential Revision: https://secure.phabricator.com/D5656
2013-04-11 10:05:55 -07:00
Chad Little
57ad790de3 Hovercard tweaks
Summary: Tightens up spacing, remove some of the borders, add alpha channel, make them all blue (sorry, red green and yellow are for 'status'). If we want to do more colors just for hovercards, I have a brown and a black in the mock, but would like to try just blue for now.

Test Plan: UIExamples, Tasks, People, Diffs, and Pastes.

Reviewers: epriestley, AnhNhan, btrahan

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5609
2013-04-06 21:16:55 -07:00
Jakub Vrana
3231df7625 Deprecate 'maniphest.enabled' and 'phriction.enabled'
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
2013-04-06 11:39:59 -07:00
Jakub Vrana
3391e3d34b Use (a = ? AND b = ?) instead of (a, b) IN (?, ?)
Summary: MySQL is not able to use indexes with searching for tuples.

Test Plan:
Explained the query before and after, saw `key_len` 16 instead of 8.
Also saw time 0.0 s instead of 2.9 s (but that was probably caused by warming up).

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5580
2013-04-05 23:02:06 -07:00
Anh Nhan Nguyen
b951a38a07 Adding hovercard event listeners for Users, Revisions, Conpherence and Commits
Summary:
Refs T1048

Adding Differential Hovercard EventListener

Adding People Hovercard EventListener

Adding basic Diffusion hovercard

Adding Conpherence Hovercard EventListener

Test Plan:
Used in a combo with working hovercards. So beautiful.

Also visited test page. Works alright.

awesometown

Reviewers: epriestley, chad, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1048

Differential Revision: https://secure.phabricator.com/D5576
2013-04-05 17:01:54 -07:00
Anh Nhan Nguyen
b0d408c5d3 Fix failing unit test testParentEdgeCases under Windows
Summary: Noticed that this one was failing under Windows for the test cases where the root path (`/`) was supposed to be returned. Was returned Windows-style, made it return UNIX-style. All others work fine (return slashes as-is).

Test Plan:
`arc unit --everything` before and after this patch on Windows.

Will try out Ubuntu in near future.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin, vrana

Differential Revision: https://secure.phabricator.com/D5497
2013-04-02 09:01:33 -07:00
Nick Harper
3f708710eb Don't barf on bad commit identifiers
Summary:
If someone provides an invalid svn rev number (like providing a git commit
hash instead) for a diffusion commit, we should ignore it like we ignore
other bad input to DiffusionCommitQuery, instead of barfing.

Test Plan:
put an invalid blame rev with rEsomehash (where E is an svn repo), and
differential loads.

Reviewers: epriestley, wez

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5389
2013-03-19 15:30:16 -07:00
epriestley
855e085c6f Uninstall Conduit calls when uninstalling applications
Summary: Fixes T2698. When applications are installed, their Conduit calls should drop out. This will also let us land Releeph without exposing Conduit calls.

Test Plan:
  - Viewed Conduit console; uninstalled some applications and verified their calls dropped out.
  - Tried to make an uninstalled call; got an appropriate error.

Reviewers: edward, btrahan

Reviewed By: edward

CC: aran

Maniphest Tasks: T2698

Differential Revision: https://secure.phabricator.com/D5302
2013-03-13 07:09:05 -07:00
epriestley
4c914a5c49 Remove all calls to renderSingleView() and deprecate it
Summary: After D5305, this method does nothing since we automatically figure out what we need to do.

Test Plan:
- Viewed a page with the main menu on it (MainMenuView).
- Viewed a revision with transactions on it (TransactionView).
- Viewed timeline UIExample (TimelineView, TimelineEventView).
- Viewed a revision (PropertyListView).
- Viewed a profile (ProfileHeaderView).
- Viewed Pholio list (PinboardView, PinboardItemView).
- Viewed Config (ObjectItemView, ObjectItemListView).
- Viewed Home (MenuView).
- Viewed a revision (HeaderView, CrumbsView, ActionListView).
- Viewed a revision with an inline comment (anchorview).
- Viewed a Phriction diff page (AphrontCrumbsView).
  - Filed T2721 to get rid of this.
- Looked at Pholio and made inlines and comments (mockimages, pholioinlinecomment/save/edit).
- Looked at conpherences.
- Browsed around.

Reviewers: chad, vrana

Reviewed By: chad

CC: edward, aran

Differential Revision: https://secure.phabricator.com/D5307
2013-03-09 13:52:41 -08:00
vrana
4e535b03e5 Adjust the shade of green in blame
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
2013-03-08 10:49:45 -08:00
vrana
b3a63a62a2 Introduce PhabricatorEmptyQueryException
Summary: It's dumb to execute a query which we know will return an empty result.

Test Plan: Looked at comment preview with "11", didn't see "1 = 0" in DarkConsole.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5177
2013-03-06 19:22:00 -08:00
vrana
1091dc7aa1 Save blame info to lint messages
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
2013-03-06 16:19:01 -08:00
vrana
e57f209056 Load blame in Diffusion by AJAX
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
2013-03-06 07:44:45 -08:00
epriestley
8ae718c2aa Require a viewer for Remarkup rendering
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
2013-03-04 12:33:05 -08:00
vrana
2cd4f2e827 Resolve SVN commit identifier inside Git in lint saver
Test Plan:
  $ git svn find-rev r11

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5173
2013-03-04 11:30:08 -08:00
vrana
0ff9ce6963 Don't try to highlight 11 in Remarkup
Test Plan: 11

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5181
2013-03-01 15:49:01 -08:00
epriestley
f6bba771cb Fix DiffusionBrowseQuery->setUser() vs ->setViewer()
Auditors: vrana
2013-02-28 17:26:43 -08:00
epriestley
0a069cb55a Require a viewer to load handles
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
2013-02-28 17:15:09 -08:00
epriestley
1539b21b45 Fix overescaping for Repositories home screen
Summary: Fixes T2623.

Test Plan:
{F34157}

{F34158}

Reviewers: chad, vrana

Reviewed By: chad

CC: aran

Maniphest Tasks: T2623

Differential Revision: https://secure.phabricator.com/D5156
2013-02-28 08:47:51 -08:00
epriestley
88497bf7df Use reverse(x::y) instead of y::x in Mercurial history queries
The revset "x:0" works, but the revset "x::0" is empty. We actually want "reverse(0::x)".

Auditors: DurhamGoode
2013-02-27 20:08:54 -08:00
epriestley
e2c9ebdbc1 Fix DiffusionMercurialHistoryQuery for file history
Summary: When querying history of a path, we should continue past branchpoints. See D5146 for more discussion.

Test Plan:
Viewing history of a file on a branch which never modified the file no longer fatals.

(Arguably we could render something like "this file was never modified on this branch" and maybe link to the branch where the branchpoint stems from, but that seems of limited use.)

Reviewers: DurhamGoode, vrana, chad

Reviewed By: DurhamGoode

CC: aran

Differential Revision: https://secure.phabricator.com/D5148
2013-02-27 19:17:26 -08:00
epriestley
e5122877a5 Fix Mercurial "Last Modified" query
Summary:
To determine when a file was last modified, we currently run `hg log ... -b branch ... file`. However, this is incorrect, because Mercurial does not interpret "-b x" as "all ancestors of the commit named x" like Git does, and we don't care about where the modification happened anyway (we always have a resolved commit as a starting point). I think this got copy-pasta'd from the History query.

Instead, drop the branch-specific qualifier and find the last modification, period.

Test Plan: Mercurial commit views of commits not on the repository's default branch are no longer broken.

Reviewers: DurhamGoode, vrana, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D5146
2013-02-27 18:41:29 -08:00
epriestley
8fead36615 Implement DiffusionMercurialContainsQuery in Diffusion
Summary:
Currently, we have no implementation, so all Mercurial commits show "None" for "Branches".

Instead, implement this method.

Test Plan: {F34076}

Reviewers: DurhamGoode, vrana, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D5145
2013-02-27 18:41:21 -08:00
epriestley
ed00e37f47 Fix commit policy stuff and anchor handling
Summary:
See discussion in D5121. Fixes T2615.

This might cause us more issues if anything is loading commit handles without passing a viewer, but I think I tested all of those cases.

Test Plan: Looked at feed, audit, maniphest, diffusion, differential, owners, repositories.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2615

Differential Revision: https://secure.phabricator.com/D5139
2013-02-27 10:54:39 -08:00
epriestley
fe78944c9d Prepare Diffusion for hovercards
Summary:
Move Diffusion to be hovercard-ready, and expand our ability to resolve commit references.

  - Link unqualified hashes of 7 characters or more which match a commit.
  - Link qualified hashes of 5 characters or more which match a commit.
  - Support `{...}` syntax.

Test Plan: {F33896}

Reviewers: chad, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5121
2013-02-27 08:04:54 -08:00
epriestley
fe500f4268 Pre-prepare for hovercards
Summary:
D5120 and followups refactor and generalize object references in Remarkup -- notably, they move remarkup rules from a central location to the implementing applications.

Preserve blame by doing moves/renames only first. This change moves application remarkup rules into those applications, and renames the ones D5120 modifies.

Test Plan: Typed some preview text into a textarea, got a valid Remarkup render.

Reviewers: vrana, chad

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5123
2013-02-26 14:57:41 -08:00
Anh Nhan Nguyen
58ef3be1c6 made DiffusionHomeController use commit summaries
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
2013-02-26 07:12:44 -08:00
vrana
e9b5eb8609 Fix number of lint count displayed on SVN files
Test Plan: Looked at it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5064
2013-02-22 09:33:18 -08:00
epriestley
b32bfb6541 Render commit summaries when rendering handles
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
2013-02-21 15:09:35 -08:00
vrana
6cee50d4d6 Link Browse Repository from repo overview
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
2013-02-21 14:45:22 -08:00
vrana
4f5e57283f Fix dynamic string usage as safe input
Summary: I somehow missed it.

Test Plan: /diffusion/PF/lint/master/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4801
2013-02-21 00:53:34 -08:00
kwadwo
762ace810d Allow files to TTL and and be garbage collected
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
2013-02-20 13:38:36 -08:00
epriestley
a22bea2a74 Apply lint rules to Phabricator
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
2013-02-19 13:33:10 -08:00
Evan Priestley
dea1a9585c Merge pull request #237 from dmrenie/master
Fix 'View Full Commit History' link
2013-02-14 07:25:47 -08:00
Evan Priestley
60cb9e1cfb Merge pull request #267 from taichi/escape_file_path
escape svn repository file paths.
2013-02-14 07:00:29 -08:00
epriestley
ef7f16180c Restore merge of phutil_tag. 2013-02-13 14:51:18 -08:00
epriestley
73cce6e131 Revert "Promote phutil-tag again"
This reverts commit 8fbabdc06d, reversing
changes made to 2dab1c1e42.
2013-02-13 14:08:57 -08:00
epriestley
4bd2ad9270 Merge branch 'master' into phutil_tag
Auditors: vrana
2013-02-13 12:42:57 -08:00
vrana
718d22d607 Convert Remarkup to safe HTML
Test Plan: None.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4919
2013-02-13 12:34:49 -08:00
vrana
5ad526942b Convert AphrontPanelView to safe HTML (except children)
Summary: Fixes some double escaping and potential XSS.

Test Plan: Looked at homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4917
2013-02-13 10:30:32 -08:00
taichi
21ddd3a73f escape svn repository file paths. 2013-02-13 19:30:11 +09:00
vrana
80fb84bd94 Convert PhabricatorTransactionView to safe HTML
Test Plan: Looked at revision detail with comments.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4915
2013-02-11 19:01:20 -08:00
vrana
868ca71451 Fix some HTML problems
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
2013-02-11 18:18:26 -08:00
vrana
c9ab1fe505 Return safe HTML from all render()
Summary:
This is pretty brutal and it adds some `phutil_safe_html()`.
But it is a big step in the right direction.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4905
2013-02-11 18:18:18 -08:00
vrana
37b98450a5 Replace array_interleave() by phutil_implode_html()
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
2013-02-11 15:27:43 -08:00
vrana
a22ef4e9b4 Kill most of phutil_escape_html()
Summary:
This resolves lots of double escaping.
We changed most of `phutil_render_tag(, , $s)` to `phutil_tag(, , $s)` which means that `$s` is now auto-escaped.
Also `pht()` auto escapes if it gets `PhutilSafeHTML`.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4889
2013-02-11 15:27:38 -08:00
epriestley
ca0d6aca10 Add separate exception for when the repository clone is unreadable.
Summary: Show a more specific exception when the local clone cannot be read because of permission issues.

Test Plan: Create a repository in an unreadable location and check for the right exception.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2368

Differential Revision: https://secure.phabricator.com/D4868
2013-02-11 08:35:00 -08:00
vrana
9b8da73765 Convert AphrontTableView to safe HTML
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
2013-02-09 15:11:38 -08:00
vrana
059920c2da Convert AphrontErrorView to safe HTML
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
2013-02-07 17:26:01 -08:00
epriestley
11bb8db970 Merge branch 'master' into phutil_tag
(Sync.)
2013-02-07 08:08:01 -08:00
vrana
2f508bf0dc Delete some phutil_safe_html()
Test Plan: Displayed revision.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4829
2013-02-05 15:52:48 -08:00
Bob Trahan
1d0058abcf Update PeopleMenu to only show integration with applications if they are installed
Summary: do so via event engine. note different order now...

Test Plan: toggled "show beta applications" to off and noted that Conpherence disappeared. Otherwise noted that links showed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2424

Differential Revision: https://secure.phabricator.com/D4708
2013-02-05 13:46:02 -08:00
epriestley
8f1311bbc1 Merge branch 'master' into phutil_tag
(Final sync.)
2013-02-05 10:23:16 -08:00
epriestley
af1f57b37a Add a preference to completely disable the file tree
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
2013-02-04 17:00:27 -08:00
vrana
34c51a61b5 Delete preference for Diffusion symbols
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
2013-02-04 11:38:22 -08:00
epriestley
0f1bdbe147 Merge branch 'master' into phutil_tag
(Sync.)
2013-02-04 06:19:52 -08:00
vrana
5459af3bdd Fix dynamic string usage as safe input
Test Plan:
  $ arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4796
2013-02-02 16:20:29 -08:00
vrana
6e95901161 Convert phutil_render_tag() to phutil_tag() for inline comments
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
2013-02-02 05:15:30 -08:00
vrana
01236dcaf0 Use PhutilNumber in translations
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
2013-01-31 09:11:01 -08:00
epriestley
7f43826854 render_tag -> tag: fix more callsites (more view, misc)
Summary: Fixes even more callsites.

Test Plan: See inlines.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4749
2013-01-31 09:08:02 -08:00
epriestley
5256731262 Don't show changes for commits which affect more than 1,000 files
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
2013-01-30 12:01:49 -08:00
epriestley
c1bcccb227 Always render comment panel in Diffusion commit view
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
2013-01-30 12:01:07 -08:00
epriestley
40547030a5 render_tag -> tag: PropertyListView
Summary: Converts callsites in PropertyListView (addDetail() and setTextContent()).

Test Plan: Grepped for PhabricatorPropertyListView, addDetail() and setTextContent().

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4695
2013-01-29 11:01:47 -08:00
epriestley
edfcd7bd2d render_tag -> tag: phame, remarkup
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
2013-01-28 18:44:15 -08:00
epriestley
fb6dbd7d3a Convert more render_tag -> tag
Summary: Mostly straightforward.

Test Plan: Browsed most of the affected interfaces.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4687
2013-01-28 18:41:43 -08:00
epriestley
a1ff679f41 Fix AphrontCrumbView (phutil_tag)
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
2013-01-25 17:07:07 -08:00
epriestley
3093d1663d Add javelin_tag(), convert easy callsites
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
2013-01-25 12:57:17 -08:00
vrana
fc6838b890 Fix double escaping after D4638
Auditors: epriestley
2013-01-25 12:05:03 -08:00
vrana
3c1b8df8ae Convert simple phutil_render_tag() to phutil_tag()
Summary: Done manually.

Test Plan: Loaded homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4509
2013-01-24 19:30:50 -08:00
vrana
20768d65d5 Convert phutil_render_tag(X, Y, '...') to phutil_tag
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
2013-01-24 19:20:27 -08:00
vrana
48561a8b1f Convert phutil_render_tag(X, Y, phutil_escape_html(Z)) to phutil_tag
Summary:
Created with spatch:

  lang=diff
  - phutil_render_tag
  + phutil_tag
    (X, Y,
  - phutil_escape_html(
    Z
  - )
    )

Test Plan: Loaded homepage

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4501
2013-01-24 19:08:55 -08:00
vrana
f8dbfdd59d Convert phutil_render_tag(X, Y) to phutil_tag
Summary:
Created with spatch:

  lang=diff
  - phutil_render_tag
  + phutil_tag
    (X, Y)

(and null manually)

Test Plan: Loaded homepage

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4500
2013-01-24 19:08:54 -08:00
vrana
c9870b12ae Don't add trailing slash to Search Owners link
Test Plan: Clicked it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4609
2013-01-24 10:33:13 -08:00
vrana
b3fa5492b4 Allow blaming of seemingly binary files in SVN
Summary:
Fixes T2388.
We check for binarity later.

Test Plan: Blamed file with 'application/x-shellscript' MIME type.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2388

Differential Revision: https://secure.phabricator.com/D4605
2013-01-23 15:22:03 -08:00
vrana
ffd46df597 Avoid error in blaming empty file
Summary: Fixes T2389, resolves TODO.

Test Plan: Blamed seemingly binary file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2389

Differential Revision: https://secure.phabricator.com/D4604
2013-01-23 15:21:08 -08:00
epriestley
70a2a653ff Revert D4359 and apply a better fix
Summary:
In D4359 I fixed an error with 'lint' in SVN repositories, but created an error with the 'lint' column in Javascript. Specifically, when we load the column information over Ajax, we now always include a 'lint' key, even if there is no lint column.

Instead, access the 'lint' property conditionally (so SVN works) but don't include the key if there's no data (so Javascript works).

Test Plan: Loaded SVN, non-SVN non-lint, non-SVN+lint repositories. Everything appeared to work correctly.

Reviewers: asherkin, codeblock

Reviewed By: codeblock

CC: aran

Differential Revision: https://secure.phabricator.com/D4578
2013-01-22 12:26:52 -08:00
epriestley
1f7e9bcadd Don't throw an exception for partially imported commits
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
2013-01-21 07:45:42 -08:00
Debarghya Das
b801ca8e6f Author Can Close Audit Option
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
2013-01-18 17:54:26 -08:00
vrana
00f730d6e9 Delete unused code in Diffusion browse file
Test Plan: Browsed a file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4508
2013-01-18 08:37:52 -08:00
epriestley
0e612c910b Sort repositories in Diffusion by name, not creation order
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
2013-01-16 10:51:08 -08:00
epriestley
b04a6a1999 Diffusion / MetaMTA options
Summary: Implement Diffusion MetaMTA options. Also make the fake '{{config.option}}' rule work, and use Remarkup to render summaries as well as descriptions.

Test Plan: Looked at Diffusion rules, edited some, looked at setup issues, verified '{{config.option}}' linked to the right option.

Reviewers: codeblock, btrahan

Reviewed By: codeblock

CC: aran

Maniphest Tasks: T2255

Differential Revision: https://secure.phabricator.com/D4466
2013-01-16 09:08:13 -08:00
vrana
f74c2bb138 Optimize displaying info about lint messages
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
2013-01-15 17:59:06 -08:00
Chad Little
4c231486d7 More Diffusion panel updates.
Summary: In Commit Details, remove the panel backgrounds.

Test Plan: Chrome

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4454
2013-01-15 15:05:15 -08:00
Chad Little
e2e890672a Adds a new 'setnobackground' panel class, implements in Differential / Diffusion.
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
2013-01-14 16:40:04 -08:00
epriestley
df2c811a54 Fix an error in DiffusionBrowseTableView with SVN repsositories with no lint information
Summary: If the repository has no lint information, we don't set a 'lint' key, but try to access it unconditionally later on.

Test Plan: Looked at an SVN repository browse view, saw no errors.

Reviewers: vrana, btrahan, codeblock

Reviewed By: codeblock

CC: aran

Maniphest Tasks: T2281

Differential Revision: https://secure.phabricator.com/D4359
2013-01-08 09:50:03 -08:00
epriestley
0ecfb75101 Expand abbreviated Mercurial hashes to full hashes
Summary:
If you go to `/rXnnnn` in Git, we expand the hash. If you go to `/rXnnnn` in Mercurial, we give you a confusing error message.

Reconcile Mercurial behavior with Git. Fixes T2265.

Test Plan: Viewed partial hash, full hash commit in Diffusion. Viewed very short hash, got reasonable behaviors.

Reviewers: btrahan, tido

Reviewed By: tido

CC: aran

Maniphest Tasks: T2265

Differential Revision: https://secure.phabricator.com/D4330
2013-01-03 06:01:53 -08:00
epriestley
f6b1964740 Improve Search architecture
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
2012-12-21 14:21:31 -08:00
vrana
2cc7f82ece Move Conduit methods inside applications
Test Plan:
/conduit/
/conduit/method/arcanist.projectinfo/
Call method

  $ echo '{}' | arc call-conduit user.whoami

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4268
2012-12-21 12:21:59 -08:00
vrana
3452315c7c Save absolute path when linting git-svn repo
Test Plan: Dumped `$this->svnRoot` in git-svn repo.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4255
2012-12-20 18:13:44 -08:00
vrana
ebe2a58f8a Use user's timezone in repo history
Test Plan: Looked at repo overview, repo history and commit detail.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4250
2012-12-20 15:16:05 -08:00
vrana
ef214e94ce Move setUser() to AphrontView
Summary: This is used in every other view.

Test Plan: Browsed around.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4248
2012-12-20 14:49:52 -08:00
epriestley
db89e23761 Make Repositories partially policy-aware
Summary: Small step toward repository hosting. No user-visible changes.

Test Plan: Looked at repositories in Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D4227
2012-12-19 11:07:06 -08:00
vrana
9607642414 Display raw contents of deleted file in Diffusion
Test Plan:
Display change of deleted file.
Use **Show Raw File (Left)**.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4184
2012-12-14 17:49:18 -08:00
Bob Trahan
5b15f725bd differntial and diffusion design tweaks
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
2012-12-13 10:46:41 -08:00
Bob Trahan
86a106d0b1 cowboy commit -- fixing fatal I introduced from D4174
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
2012-12-12 21:21:56 -08:00
epriestley
051276a96b Minor cleanups to Diffusion breadcrumbs
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
2012-12-12 18:40:18 -08:00
Bob Trahan
9e8387175e upgrade diffusion to use modern header UI and fix a few quirks
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
2012-12-12 17:50:42 -08:00
vrana
4f615ad2a9 Allow excluding paths from package
Summary: Resolves T2149.

Test Plan:
  $ bin/storage upgrade

# /owners/ - saw +
# /owners/package/1/ - saw +
# /owners/edit/1/ - added exclude paths, saw correct e-mail
# /rPabc123 - included paths are still highlighted and excluded not
# /owners/view/search/?path=/included/ - found
# /owners/view/search/?path=/excluded/ - not found
# owners.query - path: /included/
# owners.query - path: /excluded/
# new unit test

  PhabricatorOwnersPackage::loadAffectedPackages(
    $repository,
    array('/excluded/b.php'));

  PhabricatorOwnersPackage::loadAffectedPackages(
    $repository,
    array('/included/a.php'));

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2149

Differential Revision: https://secure.phabricator.com/D4102
2012-12-07 16:33:16 -08:00
epriestley
f306cab653 Use application icons for "Eye" menu and Crumbs
Summary:
Issues here:

  - Need an application-sized "eye", or a "home" icon for "Phabricator Home".
  - Some of the "apps_lb_2x" sliced images are the "_dark_" versions, not the light versions.
  - If you slice an application-sized "logout" (power off) icon and application-sized "help" (questionmark in circle) icon I can replace the current menu icons and nearly get rid of "autosprite".
  - To replace the icons on /applications/, the non-retina size is "4x", so we'd need "8x" for retina. Alternatively I can reduce the icon sizes by 50%.
  - The "Help", "Settings" and "Logout" items currently have a "glowing" hover state, which needs a variant (or we can drop it).
  - The /applications/ icons have a white hover state (or we can drop it).
  - The 1x application (14x14) icons aren't used anywhere right now, should they be? Maybe in the feed in the future, etc?
  - The "apps-2x" and "apps-large" sheets are the same image, but getting them to actually use the same file is a bit tricky, so I just left them separate for now.

Test Plan:
{F26698}
{F26699}

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T1960

Differential Revision: https://secure.phabricator.com/D4108
2012-12-07 13:37:28 -08:00
epriestley
20ee3003b5 Replace all instances of AphrontSideNavView with AphrontSideNavFilterView
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
2012-12-07 13:30:31 -08:00
vrana
cb45ad67a3 Revive essential commit details
Summary: Deleted by D3977.

Test Plan:
  ./reparse.php --message

Reviewers: epriestley, edward

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4090
2012-12-06 10:44:16 -08:00
epriestley
fb289a2c77 Remove some more copyright headers that survived in branches, etc., through the great header purge. See D3886.
Auditors: vrana
2012-12-05 09:27:27 -08:00
David Renie
98ee24bc10 Fix 'View Full Commit History' link to link to the history for the current branch rather than the default branch 2012-12-03 18:23:33 -08:00
vrana
3645dc2dd9 Filter all lint messages by owner
Summary: Also add link to this page.

Test Plan:
/diffusion/lint/
/diffusion/lint/?owner[0]=a (zero lint messages)
/diffusion/lint/?owner[0]=b (nonzero lint messages)
Clicked the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4069
2012-12-03 14:25:07 -08:00
vrana
8584b87df4 Display lint results for all repos
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
2012-12-03 14:12:25 -08:00
Bob Trahan
7154bea03c fixes two small diffusion bugs
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
2012-12-03 09:27:47 -08:00
vrana
b37b725f4f Use Git SVN revision in save_lint.php
Test Plan: Will run it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4055
2012-11-30 11:06:50 -08:00
Ricky Elrod
416d26b621 Allow users to set whether or not textareas are monospaced.
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
2012-11-27 14:06:42 -08:00
epriestley
f4fa968770 Fix an issue with browsing repositories that have README, etc
Summary: D3533 changed the path for files at root from, e.g., "README" to "/README", which fatals when we try to `git cat-file` it.

Test Plan:
This no longer happens:

{F24874}

Viewed a directory in browse without a trailing slash in the URL.

Reviewers: vrana, nh

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4016
2012-11-21 16:56:05 -08:00
vrana
254678d41d Delete dead code handling README
Summary: Since D2336.

Test Plan: Looked at README in Diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4015
2012-11-21 16:34:30 -08:00
Nick Harper
b5c7896b10 Fix diffusion browse queries in git
Summary:
If you try to load a directory in diffusion in a git repo that has a readme
file and you don't include a trailing slash in the path, you get an error
because we don't assemble the full paths of files correctly. This fixes that.

Test Plan: load such a directory in diffusion and see no fatal

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3533
2012-11-21 13:48:27 -08:00
vrana
abd880e30f Display correct size of binary files in Diffusion
Test Plan: https://secure.phabricator.com/diffusion/P/browse/master/webroot/rsrc/image/header_logo.png

Reviewers: btrahan, epriestley

Reviewed By: btrahan

CC: aran, Korvin

Maniphest Tasks: T1139

Differential Revision: https://secure.phabricator.com/D3961
2012-11-21 13:09:45 -08:00
vrana
b29dfe436e Fix fatal in browse file if lintCommit is invalid
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
2012-11-19 12:06:58 -08:00
vrana
f47c0a3a06 Fix Diffusion lint counts under SVN
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
2012-11-16 16:02:05 -08:00
vrana
ef8c43ac2a Simplify and optimize save_lint.php
Test Plan: Ran it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3933
2012-11-16 15:58:09 -08:00
vrana
3017a1f077 Order Diffusion lint details 2012-11-09 15:40:18 -08:00
vrana
a316a0bbb3 Define DiffusionRequest::setPath()
Summary: Resolves T2056.

Test Plan: Browsed SVN repo.

Reviewers: epriestley

Reviewed By: epriestley

CC: ced, aran, Korvin

Maniphest Tasks: T2056

Differential Revision: https://secure.phabricator.com/D3943
2012-11-09 11:17:56 -08:00
vrana
4f65d4f344 Display list of lint problems in Diffusion
Test Plan:
/diffusion/ARC/lint/master/, saw links.
/diffusion/ARC/lint/master/?lint=SPELL0, saw two links.
/diffusion/ARC/lint/master/src/lint/linter/ArcanistFilenameLinter.php?lint=SPELL0, saw link.
Clicked on everything.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3931
2012-11-09 11:09:03 -08:00
vrana
726a4912bd Allow filtering by lint code in Diffusion
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
2012-11-08 15:44:37 -08:00
vrana
168bdaa872 Display lint overview in Diffusion
Summary: I will add links from /diffusion/ARC/lint/ in future diff.

Test Plan:
/diffusion/ - clicked on lint messages link.
/diffusion/ARC/ - clicked on lint messages link.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3926
2012-11-08 15:41:54 -08:00
vrana
846a359d9a Display number of lint messages in Diffusion dir
Test Plan:
Went to https://secure.phabricator.com/diffusion/ARC/browse/.
Saw number of lint messages per dir, not terribly slow.
Went to https://secure.phabricator.com/diffusion/ARC/browse/master/src/difference/.
Saw number of lint messages per file.
Disabled lint, didn't see it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3921
2012-11-08 15:40:48 -08:00
vrana
47a184e2a5 Display saved lint messages in Diffusion browse file
Test Plan:
Looked at https://secure.phabricator.com/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php.
Saw "Show 16 Lint Messages".
Clicked on it, saw the messages.
Clicked on "Hide Lint Messages".
Went to https://secure.phabricator.com/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php;5be54e.
Saw "Switch Commit to See Lint".
Clicked on it, saw the messages.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3920
2012-11-08 15:39:44 -08:00
vrana
d91305e518 Add external arrow to Search Owners
Test Plan: Displayed it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3930
2012-11-08 15:36:59 -08:00
vrana
2137b49d16 Link repo history from repo overview commits
Test Plan: Clicked on it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3928
2012-11-08 15:36:35 -08:00
vrana
ef85f49adc Delete license headers from files
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
2012-11-05 11:16:51 -08:00
vrana
f34246a723 Hide Edit Maniphest Tasks in commit with disabled Maniphest
Test Plan: Viewed commit with enabled/disabled Maniphest.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3871
2012-11-01 17:38:26 -07:00
Bob Trahan
4edf8ae2fc make "browse in diffusion" action work for commits in branches other than master
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
2012-10-31 14:07:25 -07:00
Bob Trahan
9f51f1933f Bug fix for diffusion browse views for images and binary files
Summary: D3499 introduced some new hotness but broke these less common cases. add slightly updated ui for this.  Note we need a 'download' icon for this UI to not be horrendous.

Test Plan: viewed a binary file and an image in diffusion browse view

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Korvin

Maniphest Tasks: T1997

Differential Revision: https://secure.phabricator.com/D3849
2012-10-31 11:43:17 -07:00
epriestley
244b1302a0 Implement PhabricatorMarkupInterface in Diffusion/Audit comments
Summary: Followup to D3804. Makes Diffusion main comments (not just inlines) render properly with the modern markup pipeline.

Test Plan: Created previews and inline previews. Edited inlines. Saved comment, viewed comment. Verified caches were read and written using "Services" tab.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3805
2012-10-23 17:46:44 -07:00
epriestley
fdf90b46eb Use modern two-stage markup cache (PhabricatorMarkupInterface) in Differential
Summary:
See T1963 for discussion of the Facebook-specific hack.

Differential currently uses a one-stage cache (render -> postprocess -> save in cache) rather than the two-stage cache (render -> save in cache -> postprocess) offered by `PhabricatorMarkupInteface`. This breaks Differential comments coming out of cache for the lightbox, and makes various other things suboptimal (status of handles like @mentions and embeds are not displayed accurately).

Instead, use the modern stuff.

Test Plan:
  - Created preview comments and inlines in Differential.
  - Edited a Differential inline.
  - Submitted main and inline Differential comments.
  - Viewed and edited Differential summary and test plan.
  - Created preview comments and inlines in Diffusion.
  - Submitted comments and inlines in Diffusion.
  - Verified Differential now loads and saves to the generalized markup cache (Diffusion is close, but main comments still hold a single-stage cache).
  - Verified old Differential comments work correctly with the lightbox.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1963

Differential Revision: https://secure.phabricator.com/D3804
2012-10-23 17:33:58 -07:00
Bob Trahan
d9c6e07f2c If users are on the email to Phabricator, do not send them the Phabricator reply.
Summary: When we receive an email, figure out if any of the other tos and ccs are users. If they are, pass their phids through the stach as "exclude phids" and exclude them from getting the email.

Test Plan: used the various applications (audit, differential, maniphest) and noted emails were sent as expected.

Reviewers: epriestley, vrana

Reviewed By: vrana

CC: aran, Korvin, vrana

Maniphest Tasks: T1676

Differential Revision: https://secure.phabricator.com/D3645
2012-10-10 10:18:23 -07:00
epriestley
a18de93a8c Don't link files with no rendering changeset in DiffusionCommitChangeTableView
Summary:
When directories are added (e.g., on the `hg` initial commit, "/" is added) we don't render any diff for them but try to link to it in the table of contents.

Possibly we should render a diff saying "this directory was added", but stop it from complaining for now.

Test Plan: Looked at several hg and git commits which add directories to verify this gives us generally sensible behvior.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3663
2012-10-08 16:51:50 -07:00
Bob Trahan
6bbdd2609a Allow diffusion to load initial commits in Mercurial repositories
Summary: no parents - no problem - just diff that ish against "null"

Test Plan: initial commits were viewable in my test repos

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1689

Differential Revision: https://secure.phabricator.com/D3660
2012-10-08 16:35:34 -07:00
epriestley
78784aa1fe Group applications into groups on /applications/
Summary: So they're maybe a little easier to deal with? I'm going to take this formally to "plz @chad plz help" land.

Test Plan: {F20329}

Reviewers: btrahan, vrana, chad

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3609
2012-10-03 15:46:19 -07:00
vrana
422428a135 Declare used property 2012-10-03 13:32:53 -07:00
Aviv Eyal
52c68e1261 File view: Replace drop-down form with buttons for view options
Summary:
It always bothered me that adding/removing blame view takes two clicks.
Showing it like this saves a click for almost all transformations, and I think it feels nicer too.

I'm open to adding a user-setting for this (This form or the drop-down), but figured I'd ask first.

Test Plan:
Use buttons to switch modes - make sure text on button matches action.
Repeate with lines highlighted.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: jungejason, Two9A, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3499
2012-10-03 10:59:23 -07:00
vrana
e5e3617e46 Use ?view= in Blame previous but not in line link
Summary: I didn't notice that D3494 is revert of D3453.

Test Plan: Checked both line and Blame previous links.

Reviewers: epriestley, fdoemges

Reviewed By: fdoemges

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3566
2012-10-01 16:40:44 -07:00
vrana
b43f84d6dc Read rename information from DB instead of repository
Summary:
We already have this information for all VCSes, there's no point in parsing it again.
We currently support it only for Git and I remember there were some bugs in it.
It should also be faster.

Test Plan: Blamed previous revision of a file which was moved in SVN.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3564
2012-10-01 16:40:01 -07:00
Bob Trahan
557cc5b29c Make a PhabricatorRemarkupControl to de-duplicate code usage around adding a Remarkup reference to a TextAreaControl.
Summary: ...also makes Maniphest Task Edit Controller use this when its not appropriate to upsell email.

Test Plan: played around with each tool and verified the Remarkup reference was present

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1756

Differential Revision: https://secure.phabricator.com/D3468
2012-09-19 12:27:28 -07:00
vrana
5b6513e6e8 Pass branch to Diffusion URI in Owners and Externals
Test Plan: Visited detail of package in SVN and Git.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3514
2012-09-17 16:59:44 -07:00
vrana
eaf7aedb05 Enforce blame in Skip Past This Commit
Test Plan:
# Set default view to non-blame.
# Visit URL with '?view=blame'.
# Click //Skip Past This Commit//.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3494
2012-09-14 09:52:55 -07:00
vrana
34dfbdaf44 Open editor on highlighted line in Diffusion
Test Plan:
Edited:

  path$33
  path$33-35
  path

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3488
2012-09-13 11:22:00 -07:00
epriestley
1b7f04914c Make AphrontErrorView work on devices
Summary:
This is the last Paste UI element that doesn't work properly on tablets/phones. Make it flexible.

Also add empty states to Paste.

Test Plan: Viewed various errors, and `/uiexample/errors/`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3429
2012-09-11 09:55:27 -07:00
vrana
e18fb4406e Don't include view in Diffusion line permalink
Summary:
Including view in permalink seemed like the correct solution to me.
Instead, I delete it all the time when copying somewhere else.

Test Plan:
Clicked on the link.
Changed view on permalink.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3453
2012-09-07 08:15:40 -07:00
vrana
8ff52c0b6c Set viewer for all handles loaded in controllers
Summary:
I've replaced all `id(new PhabricatorObjectHandleData(...))->loadHandles()` by `$this->loadViewerHandles(...)`.
Lint caught one usage in a static method.

Test Plan: Displayed revision with sporadic author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3432
2012-09-04 23:14:26 -07:00
vrana
efd59322f2 Display 'away date' in blame
Summary: I've done D3432 in the hope that it will fix also this...

Test Plan: Blamed sporadic author.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3433
2012-09-04 23:14:10 -07:00
epriestley
93b0a501a4 Add local navigation to Differential
Summary:
Adds a flexible navigation menu to diffs that shows you your current position in the diff.

Anticipating some "this is the best thing ever" and some "this is the wosrt thing ever" on this, but let's see how much pushback we get? It seems pretty good to me.

Test Plan: Will attach screenshots.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1633, T1591

Differential Revision: https://secure.phabricator.com/D3355
2012-08-21 15:01:20 -07:00
vrana
8ee2a6f988 Explicitly load assets in revision list
Summary:
Rendering method shouldn't load data.
The view probably shouldn't load data either because it is a job for component (object that both loads data and displays them) but we don't have that concept in Phabricator.
This at least improves the architecture a little bit.

Test Plan: /differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: alanh, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3325
2012-08-20 18:02:20 -07:00
epriestley
46f4f6cf0a Fix a fatal if a directory listing contains a non-file called 'readme'
Summary: See T1665. If you have a directory named 'readme', we try to read it as a README.

Test Plan: Created a directory named 'readme', hit a similar fatal to the one in T1665, applied this patch, everything worked great.

Reviewers: btrahan, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1665

Differential Revision: https://secure.phabricator.com/D3312
2012-08-16 12:48:37 -07:00
Alan Huang
627584f501 Jump to PHP docs from symbol search more often
Summary:
It's kind of nice to type `s explode` in jump nav and have it
just work. This involves weakening a bunch of the request parameter
checks, but the only real extra assumption is that language defaults to
PHP... which is not that big of a stretch, and it's not like we know
about any other languages' documentation.

It'd be better to have builtins be more first-class and less awkward
hack, but that seems hard.

Test Plan: Search for symbols.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3302
2012-08-15 17:47:04 -07:00
vrana
375400802b Register commits as Diffusion fact objects
Summary:
We will need to process them.
Maybe it will fit better in Repository application but we don't have it yet.

Test Plan:
  $ ./fact cursors

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, royw

Differential Revision: https://secure.phabricator.com/D3287
2012-08-14 18:04:34 -07:00
epriestley
012370c6ab Use sprites for (nearly) all application icons
Summary: Sprites for everyone.

Test Plan: Loaded `/applications/`.

Reviewers: btrahan, chad, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3280
2012-08-14 14:23:55 -07:00
vrana
6ce39db6ca Add ID to Pending Differential Revisions panel 2012-08-13 17:24:31 -07:00
epriestley
66cee129b6 Remove all code referencing old tab navigation
Summary:
  - Add getHelpURI() to PhabricatorApplication for application user guides.
  - Add a new "help" icon menu item and skeletal Diviner application.
  - Move help tabs to Applications where they exist, document the other ones that don't exist yet.
  - Grep for all tab-related stuff and delete it.

Test Plan: Clicked "help" for some apps. Clicked around randomly in a bunch of other apps.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3267
2012-08-13 15:28:41 -07:00
David Reuss
df3162584e Convert to custom encoding for diffusion blame views
Summary:
If a repository is configured with a custom encoding, it wasn't respected by DiffusionGitFileContentQuery making all views with
non-UTF8 characters fail. Check if we have a custom encoding and encode if any it set.

NOTE: This only works for Git repositories.

Test Plan: Browsed a repository with custom encoding before and after this patch.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T452

Differential Revision: https://secure.phabricator.com/D3251
2012-08-12 08:50:49 -07:00
epriestley
d3fd790574 Add basic support for new navigation menu
Summary:
Add a new left-side application menu. This menu shows which application you're in and provides a quick way to get to other applications.

On desktops, menus are always shown but the app menu can be collapsed to be very small.

On tablets, navigation buttons allow you to choose between the menus and the content.

On phones, navigation buttons allow you to choose between the app menu, the local menu, and the content.

This needs some code and UI cleanup, but has no effect yet so I think it's okay to land as-is, I'll clean it up a bit as I start integrating it. I want to play around with it a bit and see if it's good/useful or horrible anyway.

Test Plan: Will include screenshots.

Reviewers: vrana, btrahan, chad

Reviewed By: btrahan

CC: aran, alanh

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3223
2012-08-11 07:06:12 -07:00
vrana
9030fe8b09 Respect type in symbol query
Summary: Was completely ignored.

Test Plan: /diffusion/symbol/C/?type=function

Reviewers: alanh

Reviewed By: alanh

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D3234
2012-08-09 22:28:17 -07:00
vrana
81946fc08d Join author and committer in Diffusion browse
Summary: Consistent with history view, simpler.

Test Plan: /diffusion/X/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3230
2012-08-09 18:15:58 -07:00
Bob Trahan
b86d995b40 Detect if a commit *really* doesn't exist and 4oh4 from Diffusion commit view
Summary: rather than showing an erroneous "we still parsing" message.

Test Plan: for each version control system, typed in a garbage URL and got a 4oh4. (note this actually fails for SVN -- see comment about how my code fails atm -- and DiffusionGitRequest seems to pick off this error in advance and returns an AphrontUsageException.)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1624

Differential Revision: https://secure.phabricator.com/D3201
2012-08-09 09:27:45 -07:00
Alan Huang
eeb359bae6 Make D3123 more consistent
Summary:
Put the function in the base class so all the Diffusion views
can use it. Also use shinier tooltips.

Test Plan: Browse Diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3206
2012-08-08 18:26:23 -07:00
Bob Trahan
8a4c08b01d Allow commits to be associated with projects and associated goodies
Summary:
- Commit detail view
 - List of projects
 - "edit" action which takes the user to a simple form where they can only add / remove projects.
-  Integrated the project relationship into the commit search indexer
 - fixed a bug from D790; it seems you must select the column if you're going to join against it later. Without this change searching for author or projectfails 100% for me.

Test Plan: added and removed projects. verified appropriate projects showed up in detail and edit view. searched for commits by project and found the ones I was supposed to...!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1614

Differential Revision: https://secure.phabricator.com/D3189
2012-08-08 10:03:41 -07:00
vrana
8ab1789329 Improve displaying of very large commits
Summary:
Behave like Differential.

Also save one path ID query.

Test Plan: Displayed very large commit, clicked in ToC, clicked on Load.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3176
2012-08-07 10:51:38 -07:00
Alan Huang
a8d6af0f42 Infrastructure changes to support scoped symbols
Summary:
 - import_project_symbols supports an optional extra field, which is the
   context of the symbol.
 - Symbol query can take a symbol argument, either as a parameter or a
   URL component (so you can now jump nav to `s Zerg/rush`, for
   example).
 - Conduit method not yet updated. Will do that later.

NOTE: Not providing a context is distinct from providing an empty
context, because an empty context stands for top-level context, i.e.
functions and classes for PHP. It will not find class methods, etc. It
is possible that we should use some weird token that could not normally
be a context name to stand in for empty context.

Test Plan: Do a bunch of symbol searches.

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, Korvin

Maniphest Tasks: T1602

Differential Revision: https://secure.phabricator.com/D3148
2012-08-07 09:28:49 -07:00
vrana
ceb522427e Highlight lines on drag and drop in Diffusion
Summary: Nobody knows that it's possible to highlight more lines because there's no interface for it.

Test Plan:
Highlighted:

- single line
- top to bottom
- bottom to top
- inside to outside

Reviewers: Korvin, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D3150
2012-08-05 18:56:25 -07:00
epriestley
643653dc61 Store the "current" application in the controller
Summary:
When we match an application route, select it as the current application and store it on the controller.

Move routes for the major applications into their PhabricatorApplication classes so this works properly.

Test Plan: Added a var_dump() and made sure we picked the right app for all these applications.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3144
2012-08-05 14:03:39 -07:00
epriestley
194dc40672 Add a meta-application
Summary:
  - Adds a new "Applications" application.
  - Builds an application list via application config instead of via hard-coding, so we can move toward better concepts of installing/uninstalling applications, etc.
  - Applications indicate that they need attention with notice counts and brief status messages rathern than 50 giant tables of all sorts of app data.

I want to try replacing the home screen with this screen, pretty much. Not sure if this is totally crazy or not. What does everyone else think?

Test Plan: Will add screenshots.

Reviewers: btrahan, chad, vrana, alanh

Reviewed By: vrana

CC: aran, davidreuss, champo

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3129
2012-08-02 14:07:21 -07:00
Alan Huang
ce8bcf887d Render an edit link in Diffusion directory views
Summary:
If the user has an editor configured, an Edit link appears next
to the History link.

Somewhat suboptimally, the column is still there if there are no edit
links, it's just empty. I don't know if it matters...

Test Plan: Load Diffusion pages while changing editor setting in preferences.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, avivey

Maniphest Tasks: T1558

Differential Revision: https://secure.phabricator.com/D3124
2012-08-02 12:22:50 -07:00
Alan Huang
6e1f5e353a Abbreviate author/committer in Diffusion directory views
Summary:
The tables are currently kind of wide, and the emails for
non-recognized users seem like the least useful parts, so I put them in
tooltips so they're only visible if you want them. On the other hand, it
means you can only view one at a time, and if you're on mobile you can't
see them at all. Overall, not sure whether this is a good idea.

Test Plan:
Load Diffusion pages with some recognized and some
unrecognized users. Hover over the latter and see that emails appear.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3123
2012-08-01 14:59:32 -07:00
epriestley
a476e5c08d Include symbols in main typeahead
Summary:
  - Include symbols in main typeahead results.
  - Simplify the symbol query a bit and extend PhabricatorOffsetPagedQuery. There was some stuff around language ranking that I got rid of, I think the theory there was that mapping file extensions to languages might not work in general but I think it works fine in practice, and we have more config stuff now around guessing languages and getting the mappings right.
  - Make it easier to debug the typeahead by showing the results in page format for non-ajax requests.
  - When we have too many results, show only the top few of each type.

Depends on D3116, D3117

Test Plan: Used typeahead, got symbols in results. Hit endpoint with non-ajax, got useful debug view.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3118
2012-08-01 12:36:47 -07:00
vrana
606ef34d61 Load commit branches and tags by AJAX
Summary:
Each query takes over 2 seconds in FBCODE.
I didn't find a way how to speed them up.
There's also no easy way how to parallelize them at least.
So AJAX is the last instance.

Test Plan: Loaded commit with one branch and no tag.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3112
2012-07-31 17:01:41 -07:00
Bob Trahan
c8578b9fa8 Add a link to repository tool if there are no configured repositories
Summary: helping noobs help themselves

Test Plan: set $rows = array() and verified the txt. also threw a false && for my isAdmin conditional to check the other txt

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1086, T1360

Differential Revision: https://secure.phabricator.com/D3100
2012-07-30 09:09:40 -07:00
vrana
05bf6bef81 Jump to correct line in Blame previous revision
Test Plan: Jumped on correct line in SVN, Git and HG repos.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3084
2012-07-27 13:38:15 -07:00
Nick Harper
14e3e5ccfd Provide user option for disabling diffusion symbol cross-references
Summary: Some people don't like these, so they should be able to turn them off.

Test Plan:
Toggled the setting on and off; loaded a page in diffusion and differential
that should have symbol cross-references, and saw that they weren't linked
when I had the setting disabled. I also checked that the symbols are still
linked when the setting hasn't been touched.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3069
2012-07-26 20:25:27 -07:00
vrana
0cfdf2f74f Allow specifying against commit in DiffusionRawDiffQuery
Summary: I will need this for tracking line number in Blame previous revision.

Test Plan:
  $ hg diff --rev 0:1
  $ svn diff -r 64382:64383 README

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3083
2012-07-26 15:13:03 -07:00
vrana
636877807f Fix typo in comment 2012-07-26 13:45:48 -07:00
epriestley
c85d6d5e1b Fix Diffusion fatal with symbol highlighting
Summary: This returns a list of PHIDs, not objects which need to be pulled.

Test Plan:
Repro'd fatal locally, verified it was fixed.

Repro steps are:

  - Create an arc project associated with a repository, with indexed language(s) and subprojects.
  - View a file in that repository.

Reviewers: nh, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3038
2012-07-23 12:49:43 -07:00
Bob Trahan
bc29a3e8a2 Make inline comment preview work in Diffusion
Summary: created a PhabricatorInlineCommentPreviewController so controllers in Diffusion and Differential respectively just have to handle the URI mapping and data loading like good little controllers.

Test Plan:
left inline comments on commits, deleted inline commits, submitted inline comments -- all worked well
did the same on some diffs

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1176

Differential Revision: https://secure.phabricator.com/D3034
2012-07-23 11:01:28 -07:00
Bob Trahan
ac46ea39b8 Make it so the diffusion comment panel can be haunted
Summary: ...basically by pounding the DOM a bit to be a little closer to differential. I also make the "add comment" UI show up if and only if the commit is rendering properly.

Test Plan: prezzed "Z" on diffusion and noted it was like differential now

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1177

Differential Revision: https://secure.phabricator.com/D3027
2012-07-23 09:15:10 -07:00
Nick Harper
0a3e49cd76 Show cross-references in diffusion
Summary:
We do this in differential. To do this in diffusion, we need to know the
arcanist project (which I do by loading all possible projects for the
repository) and the language.

Test Plan:
load a php file in diffusion to see crossreferences; load a text file and
check darkconsole that it didn't try to crossreference.

Reviewers: epriestley, vrana, jungejason

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3020
2012-07-19 22:01:31 -07:00
vrana
43b94a6f95 Redirect after post when changing Diffusion view
Summary: I spend most time in software development by being lazy.

Test Plan: Changed view, reloaded page, viewed the file without sending the form.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3012
2012-07-19 10:27:53 -07:00
vrana
91d8b3c8ab Highlight audited paths in commit detail
Summary:
This doesn't indicate which path is part of which package - I think it would be too heavy.
It just highlights the paths in a similar way as audits are highlighted.

Maybe we can use different colors for highlighting different packages and use them also in paths. We can mix the colors if one path is part of more packages :-).

Test Plan:
Viewed commit with 9 files and 4 packages where I am responsible only for one of them.
Verified that the only file in my package is highlighted.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, haugen

Maniphest Tasks: T1226

Differential Revision: https://secure.phabricator.com/D2982
2012-07-18 18:02:05 -07:00
vrana
0d162e15f6 Make Diffusion view sticky
Summary: Blame can be slow.

Test Plan:
Viewed a file with no preference, saw blame.
Changed view, saw it.
Viewed a file, saw the changed view.
Viewed a file as raw document.

Reviewers: Two9A, epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin, btrahan

Maniphest Tasks: T1278

Differential Revision: https://secure.phabricator.com/D3000
2012-07-17 17:38:29 -07:00
vrana
5daf08048b Fix path autocomplete
Summary: Paths autocompleter sometimes omits `/`.

Test Plan:
# Go to https://secure.phabricator.com/owners/new/.
# Write `/specs/h` to path.
# Delete the second slash resulting in `/specsh`.
# Don't find `/specshistorytest.html` in autocomplete.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2981
2012-07-16 13:28:16 -07:00
epriestley
ff4774a970 Don't fatal on missing commit in DiffusionBrowseFileController
Summary:
Still not 100% sure what the repro case here is, but we do something similar on line 438 above. One case could be an SVN repo with a subpath specified, I think.

In any case, don't fatal if we're missing the commit.

Test Plan: Loaded some browse views, although I don't have a repro.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2959
2012-07-11 17:02:15 -07:00
epriestley
176b4a68a8 Fix some mercurial edge cases
Summary:
  - Old versions of Mercurial give different output for `hg log -- ''` and `hg log`. Just use `hg log`.
  - Branch names with spaces can't be specified in `--rev`. I talked with hstuart in #mercurial and apparently am not crazy.

Test Plan:
  - Viewed history of a repository.
  - Viewed history of a file.
  - Viewed branch `m m m m m 2:ffffffffffff (inactive)`
  - Learned that you checkout this branch with `hg checkout ':m m m m m 2:ffffffffffff (inactive)'`

Reviewers: dschleimer, btrahan

Reviewed By: dschleimer

CC: aran

Maniphest Tasks: T1268

Differential Revision: https://secure.phabricator.com/D2950
2012-07-10 10:36:33 -07:00
epriestley
3e15c3580d Simplify build file from data-or-hash code
Summary:
We have a bit more copy-paste than we need, consolidate a bit.

(Also switch Mercurial to download git diffs, which it handles well; we use them in "arc patch".)

Test Plan:
  - Downloaded a raw diff from Differential.
  - Downloaded a raw change from Diffusion.
  - Downloaded a raw file from Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2942
2012-07-09 10:38:25 -07:00
Miles Shang
1b8ed98ddc Multi-line highlighting in Diffusion.
Summary: Currently, Diffusion supports highlighting of line ranges passed in the URI. It would be helpful to be able to highlight multiple line ranges.

Test Plan: Accessed directly via URL in my sandbox. Seems to work. I'm not sure what other components use this functionality, but this change should be backwards compatible.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D2921
2012-07-05 19:44:12 -07:00
epriestley
4dd5bcf1cd Don't fail in Diffusion if .gitmodules is missing
Summary:
See T1448. If this file isn't present, just move on instead of failing, since it's a (sort of) legitimate repository state.

Also fix some silliness a little later that got introduced in refactoring, I think.

Test Plan: Added an external to my test repo and removed ".gitmodules". Verified that the directory is now viewable after this patch.

Reviewers: btrahan, davidreuss, jungejason

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1448

Differential Revision: https://secure.phabricator.com/D2922
2012-07-05 06:12:35 -07:00
Jason Ge
61b79b5359 Use binary_safe_diff from arcanist
Summary:
binary_safe_diff is needed in arcanist too. Moved it over to
arcanist. See D2915.

Test Plan: diffusion page rendered correctly on binary file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2916
2012-07-03 13:51:37 -07:00
David Reuss
fd94700d69 Minor, really fix empty reference properties in diffusion
Summary:
Oh man, after the explode/map, the output when no references is actually

  array(1) {
    [0]=> string(0) ""
  }

.. making the falsey check fail to work as expected.

Test Plan: same as D2892, but with a little more scrutiny :p

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2893
2012-06-30 06:43:33 -07:00
David Reuss
19d58564d1 Minor, don't show references in diffusion if there isn't any
Summary:
When there are no other references (and there will be none ususally,
if D2891 is accepted), we would show "References:" with empty contents.

Avoid that by testing for empty output after applying stupid filtering.

Test Plan: Looked in a standard repository with no special refs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2892
2012-06-30 06:26:36 -07:00
David Reuss
30b06c1ad0 Diffusion: turn branches/refs properties into links
Summary: They just beg to be clicked.

Test Plan: clicked furiously with my mouse until i got tired - went expected places.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2889
2012-06-29 14:14:15 -07:00
vrana
d7b8bc892b Display revision number in history
Test Plan:
Displayed repository.
Displayed repository history.
Wondered that we actually have bunch of commits without a revision.
Displayed blame.
Didn't display merge commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2840
2012-06-22 17:10:45 -07:00
vrana
a7a978d1f3 Link revision from blame
Summary: I need this much more than commit but it can be useful too.

Test Plan: Displayed blame, clicked on the link.

Reviewers: epriestley

Reviewed By: epriestley

CC: john, aran, Korvin, phunt

Differential Revision: https://secure.phabricator.com/D2820
2012-06-22 12:48:46 -07:00
Jason Ge
112acf11cf Enable 'jumping to toc' on diffusion commit page
Summary:
Diffusion page is sharing the keyboard shortcuts code with
Differential page. But since the toc (Changes) panel doesn't have id
'differential-review-toc', the 'jumping to toc' doesn't work. The fix is
to add the ID. I don't like adding 'Differential' to the Diffusion page.
Later we should refactor the code to extract the shared components out of Differential.

Test Plan:
verified that 't' worked on the diffusion commit page.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: hwang, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2500
2012-06-18 11:09:25 -07:00
Nick Harper
865680ad30 Don't throw an exception if the author of a tag can't be found
Summary:
It's possible to have a tag with no tagger (or git used to allow this),
so some tags (like 26791a8bcf0e6d33f43aef7682bdb555236d56de in the linux
kernel) were causing trouble.

Test Plan:
opened diffusion to a page where I was previously getting a red box complaining
about being unable to parse the output of git for-each-ref.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, vrana

Differential Revision: https://secure.phabricator.com/D2711
2012-06-11 16:53:52 -07:00
vrana
bed3a9817b Merge author and committer
Summary: These columns holds the same value in most cases which irritates me.

Test Plan: Displayed history with same author and committer, emulated different committer.

Reviewers: hsb, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2671
2012-06-07 17:33:34 -07:00
Two9A
2d52881d4e Diffusion/BrowseFile: Set highlighted with blame as default view
Summary: As per title: when browsing files in Diffusion, set "Highlighted with blame" as the default view instead of falling back to "Highlighted".

Test Plan: Tested view with a local Diffusion setup, defaults correctly.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1278

Differential Revision: https://secure.phabricator.com/D2669
2012-06-07 16:56:26 -07:00
vrana
a8b5ca63bf Support Git renames in the middle of path
Test Plan: Blame previous revision with such rename.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2670
2012-06-07 12:00:44 -07:00
vrana
ec9589fb3b Ignore errors in svn diff
Summary: Otherwise attaching the commit diff doesn't work.

Test Plan: Reparsed previously failing commit message.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2605
2012-06-01 21:45:33 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2612
2012-05-30 16:57:21 -07:00
epriestley
09c8af4de0 Upgrade phabricator to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
vrana
af6238ca4a Inform about changes made between last revision and commit
Summary:
This adds a link to [Closed] e-mail if it detects some changes.
It compares added and removed lines with 3 lines context.
The subtle form of informing is permissive to false negatives and positives.
I have an e-mail filter for [Closed] e-mails so I wouldn't personally notice this change - we should probably promote this feature a little bit.

Test Plan:
Reparse a diff with a change after last update.
Reparse a diff without a change after last update.

Reviewers: jungejason, epriestley

Reviewed By: jungejason

CC: aran, Koolvin, btrahan

Maniphest Tasks: T201

Differential Revision: https://secure.phabricator.com/D2540
2012-05-25 21:39:58 -07:00
Hafsteinn Baldvinsson
a438c87c52 Show the difference between a committer and an author
Summary:
Git and hg (supposedly..) differentiate between an author (who wrote the patch)
and a committer (who applied the patch).

This patch allows Phabricator to note when a patch is committed
by someone other than the Author.

Test Plan:
Created 2 accounts,
 - U (Account with a PHID)
 - U' (Account without a PHID)
and had them create and commit commits

testing if their username/real name would be displayed correctly in Diffusion,
  - BrowserTable
  - HistoryTable
  - Code revision

Teztz,
A(uthor)/C(ommitter)
If it's A/A then Author committed

UL = User link (<a href="/p/username">username</a>)
UN = User name ("Firstname Lastname")

Tezt | Expected in table  | Got
-------------------------------------------
A/A   | UL/UL             | UL/UL
A'/C  | UN/UL             | UN/UL
A/C'  | UL/UN             | UL/UN
A'/C' | UN/UN             | UN/UN

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T688

Differential Revision: https://secure.phabricator.com/D2541
2012-05-23 08:34:54 -07:00
epriestley
a009c93350 Use "-b", not "--branch", when issuing "hg log" in Phabricator
Summary:
Mercurial renamed "--only-branch" to "--branch" about two years ago. "-b" exists in both versions.

http://www.selenic.com/pipermail/mercurial-devel/2010-April/020469.html

We have a few other cases where we use features that exist only in recent Mercurial (notably, 'ancestors' in log) but we can work around this one easily.

Test Plan: Looked at a Mercurial repo in Diffusion, verified that "log -b" commands issued and that the output was correct.

Reviewers: btrahan, Makinde, ipalaus

Reviewed By: ipalaus

CC: aran

Maniphest Tasks: T1264

Differential Revision: https://secure.phabricator.com/D2533
2012-05-22 07:14:55 -07:00
Jason Ge
2b39a77fd8 variable name incorrect
Summary: as title

Test Plan: no

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2526
2012-05-21 15:34:35 -07:00
Bob Trahan
80f479d948 Make directories with spaces in their names work OK in diffusion end to end
Summary:
we were parsing the git log output slightly incorrectly and over-exploding on spaces. we also needed to escape the path %20 stuff`.

Not sure if there's something fancy to do given folks should reparse their repos if they are impacted by this issue.

Test Plan:
made a directory with spaces and some dummy revisions. observed diffs wouldn't load and links broken.
with patch, ran scripts/reparse.php for pertinent revisions and diffs loaded and links weren't broken.

Reviewers: floatinglomas, epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1252

Differential Revision: https://secure.phabricator.com/D2510
2012-05-20 15:29:36 -07:00
epriestley
3a1ee00335 Select default branches more effectively in Diffusion
Summary: If you have an empty value saved in the "default branch" field, we default to empty string (or null, or whatever) instead of the correct default.

Test Plan:
  - Looked at a Git repo with an empty default branch, got a default to "master" instead of an error.
  - Looked at a Mercurial repo with an empty default branch, got a default to "default" instead of an error.

Reviewers: btrahan, csilvers

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1237

Differential Revision: https://secure.phabricator.com/D2512
2012-05-20 14:50:43 -07:00
Craig Silverstein
18deff557e Fix a typo.
Test Plan: (None.)

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2483
2012-05-18 17:58:58 -07:00
epriestley
820a6d407a Improve performance of repository discovery under Mercurial
Summary: Prelminary, I want to test this a bit more when I'm better rested. Provide a "bulk" import mode for Mercurial so we can do initial discovery more quickly.

Test Plan: Imported the Mercurial repository in 2m45s, blocked on MySQL I/O rather than Mercurial I/O.

Reviewers: csilvers, btrahan

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2457
2012-05-11 18:29:14 -07:00
epriestley
d24c81c5e2 Support branch offset/limit in Mercurial
Summary:
  - Support offset/limit as added by D2442, for Mercurial.
  - We just list everything and slice, no clear way to do better and this isn't a major perf issue.
  - No clear way to easily get by-update sorting, we can implement this by looking up revs if someone asks.
  - Bury some of the Git implementation details inside the Git query.

Test Plan:
  - Looked at a Git repo, clicked "View All Branches".
  - Looked at a Hg repo, clicked "View All Branches".
  - Limited page size to 1, made sure it limited properly.

Reviewers: aurelijus, btrahan, vrana

Reviewed By: aurelijus

CC: aran

Differential Revision: https://secure.phabricator.com/D2453
2012-05-10 14:18:49 -07:00
Aurelijus
ba98089426 Branch view improvements.
Summary:
Sorting by last commit date
Branch view limit to 25 branches
All branches table page with pagination on Git

Test Plan:
* Check repository view for expected behavior on branch table view
* Check all branches page & test pagination

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1200

Differential Revision: https://secure.phabricator.com/D2442
2012-05-10 11:47:46 -07:00
epriestley
1bf68e06a5 Improve Diffusion error messages and UI for partially imported repositories
Summary:
  - When you have an un-cloned repository, we currently throw random-looking Git/Hg exception. Instead, throw a useful error.
  - When you have a cloned but undiscovered repository, we show no commits. This is crazy confusing. Instead, show commits as "importing...".
  - Fix some warnings and errors for empty path table cases, etc.

Test Plan:
  - Wiped database.
  - Added Mercurial repo without running daemons. Viewed in Diffusion, got a good exception.
  - Pulled Mercurial repo without discovering it. Got "Importing...".
  - Discovered Mercurial repo without parsing it. Got "Importing..." plus date information.
  - Parsed Mercurial repo, got everything working properly.
  - Added Git repo without running daemons, did all the stuff above, same results.
  - This doesn't improve SVN much but that's a trickier case since we don't actually make SVN calls and rely only on the parse state.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T776

Differential Revision: https://secure.phabricator.com/D2439
2012-05-09 17:28:57 -07:00
Aurelijus
872845cc6c Improve branch table view with history link & other features
Summary: Add history column & history link per branch on branch table view, also later add some more features like last commit date, etc.

Test Plan: Checked if History column is in browser table view & history links are properly linked.

Reviewers: epriestley

Reviewed By: epriestley

CC: davidreuss, aran, Koolvin

Maniphest Tasks: T1201, T1202, T1200

Differential Revision: https://secure.phabricator.com/D2432
2012-05-08 15:03:06 -07:00
epriestley
d2b01aead0 Use one daemon to discover commits in all repositories, not one per repository
Summary:
See D2418. This merges the commit discovery daemon into the same single daemon, and applies all the same rules to it.

There are relatively few implementation changes, but a few things did change:

  - I simplified/improved Mercurial importing, by finding full branch tip hashes with "--debug branches" and using "parents --template {node}" so we don't need to do separate "--debug id" calls.
  - Added a new "--not" flag to exclude repositories, since I switched to real arg parsing anyway.
  - I removed a web UI notification that you need to restart the daemons, this is no longer true.
  - I added a web UI notification that no pull daemon is running on the machine.

NOTE: @makinde, this doesn't change anything from your perspective, but it something breaks this is the likely cause.

This implicitly resolves T792, because discovery no longer runs before pulling.

Test Plan:

  - Swapped databases to a fresh install.
  - Ran "pulllocal" in debug mode. Verified it correctly does nothing (fixed a minor issue with min() on empty array).
  - Added an SVN repository. Verified it cloned and discovered correctly.
  - Added a Mercurial repository. Verified it cloned and discovered correctly.
  - Added a Git repository. Verified it cloned and discovered correctly.
  - Ran with arguments to verify behaviors: "--not MTEST --not STEST", "P --no-discovery", "P".

Reviewers: btrahan, csilvers, Makinde

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T792

Differential Revision: https://secure.phabricator.com/D2430
2012-05-08 12:53:41 -07:00
epriestley
191f6d904f Fix a bug with "Resign" in Diffusion
Summary: The PHID list is a list, not a map -- I must have broken this in refactoring or something, since everything else works fine. See D2013.

Test Plan: Viewed a resignable revision, saw "Resign" (new after this commit), resignd.

Reviewers: 20after4, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2417
2012-05-07 13:37:41 -07:00
vrana
826e5405b6 Open files in very large diffs inline
Summary: This allows writing inline comments and reduces different behavior between normal and very large diffs.

Test Plan:
Verify that normal diff works.
Verify that very large diff works.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2361
2012-05-02 17:51:35 -07:00
vrana
81dd92fcdc Display Show Raw File link in Diffusion Change View
Summary: NOTE: `renderViewOptionsDropdown()` adds unnecessary parameters to URL but the link just redirects anyway.

Test Plan:
Show Raw File (Left and Right) in SVN and Git.
Verify also Added and Deleted files.

Reviewers: epriestley, aran

Reviewed By: epriestley

CC: Koolvin

Differential Revision: https://secure.phabricator.com/D2370
2012-05-02 14:18:35 -07:00
epriestley
63ce372480 Add "DiffusionRawDiffQuery"
Summary:
  - This is only slightly useful for updating Differential, since DiffQuery (vs RawDiffQuery) already gets you most of what you need. The only thing is that DiffQuery returns the diff for one path only right now(and the SVN version is very "special"). Should be easy to fix in the Git/HG cases at least, though (or maybe just use RawDiffQuery to avoid the SVN mess).
  - Added a "download raw diff" link.

Test Plan: Viewed Diffusion and raw commits for SVN, Mercurial and Git repositories.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2350
2012-05-02 13:43:45 -07:00
epriestley
321b776148 Show README on repository screen in Diffusion
Summary:
  - Show README on the repository screen.
  - Move README to the bottom of the page for both repository and browse screens.
  - Support "README.rainbow".

Test Plan: Looked at repository, browse screens. Made a "README.rainbow".

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1104

Differential Revision: https://secure.phabricator.com/D2336
2012-04-30 07:47:41 -07:00
epriestley
160ec660b0 Minor, fix an error when looking at a new, unparsed repository. 2012-04-28 16:37:41 -07:00
epriestley
2bdde748d9 Fix DiffusionGitBrowseQuery to parse with "git config -l -f" instead of PHP ini parser
Summary: See discussion on rPc0aac8267dda74664acac5c93d9aeb5a0f9c4564.

Test Plan: Looked at externals/, got a correctly-behaving link.

Reviewers: vrana, davidreuss, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2317
2012-04-27 12:51:50 -07:00
epriestley
12cd0d0b67 Handle symbolic commit names better in Diffusion
Summary: We now allow symbolic commits, so let them through the pipeline.

Test Plan: {F10571}

Reviewers: davidreuss, btrahan, vrana

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2315
2012-04-27 12:51:44 -07:00
vrana
a0417d97c8 Add assert_instances_of()
Auditors: epriestley
2012-04-25 16:23:06 -07:00
vrana
b44f687ce2 Fix link to submodule's history
Test Plan:
https://secure.phabricator.com/diffusion/P/browse/master/externals/
Click on History of `javelin/`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2316
2012-04-25 10:32:44 -07:00
epriestley
284ee03919 Show refs in Diffusion for Git commits
Summary: In Diffusion, for Git, show commit refs (like "origin/master, origin/HEAD").

Test Plan: Looked at several Git, Hg and SVN commits.

Reviewers: davidreuss, btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2311
2012-04-25 07:21:03 -07:00
epriestley
7b334f37bd Improve tag support in Diffusion
Summary:
  - When viewing a commit, show its tags.
  - For commits with many tags, show a list of all tags on the tag list interface.
  - Improve some handling of symbolic references.
  - When tags contain content, show it on the browse view reached by clicking the tag name.

Test Plan: Looked at commits with and without tags, clicked "More tags...", clicked tag names.

Reviewers: btrahan, vrana, davidreuss, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2290
2012-04-23 18:36:25 -07:00
epriestley
e9bd842227 Allow "blame previous revision" to track file moves and handle edge cases
Summary:
  - Track + message through file moves.
  - Stop + message on file create.
  - Stop + message on first commit.

Test Plan:
  - Tested blaming through a move, through a create, and through the first commit.
  - Verified this doesn't break anything in SVN / Mercurial.

Reviewers: vrana, btrahan, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1091

Differential Revision: https://secure.phabricator.com/D2295
2012-04-23 18:16:38 -07:00
David Reuss
42b1c73f41 Allow CC's/Auditors added to audits
Test Plan:
Added CC's/Auditors, clicked the form elements, and saw correct
behaviour. Verified that metadata was present in the detail table.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, 20after4, Koolvin

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2002
2012-04-23 13:50:25 -07:00
epriestley
944049d871 Add a paginated list of all repository tags to Diffusion
Summary: Now supports more than 25 tags!

Test Plan: Set page size to 1, paginated. Verified SVN / Hg don't break/explode.

Reviewers: davidreuss, vrana, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2268
2012-04-19 09:39:19 -07:00
epriestley
dec8acd803 Add very basic tag support to Diffusion
Summary: Lists the 25 most recent tags on the "Repository" page.

Test Plan: Looked at a git repository with a tag, saw it. Looked at HG/SVN repos, they didn't break.

Reviewers: davidreuss, 20after4, btrahan, vrana, jungejason

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1130

Differential Revision: https://secure.phabricator.com/D2255
2012-04-18 08:02:08 -07:00
epriestley
5f0d8e09d8 Use "user-select: none" to provide a visual cue about copy/paste JS magic
Summary:
  - For line numbers, use "user-select: none" to make them unselectable. This provides a stronger visual cue that copy/paste is enchanted.
  - In Paste, make it look sensible again after the blame-on-blame refactor in Diffusion. See also TODO to share this code formally.
  - In Diffusion, use the "phabricator-oncopy" behavior.

NOTE: I left blame/commit columns selectable in Diffusion, since you might reasonably want to copy/paste them?

NOTE: In Differential, the left side of the diff still highlights, even though it will be copied only if you select part of a line on the left and nothing else. But this seemed like a reasonable behavior, so I left it.

Test Plan:
  - Looked at Paste. Saw a nice line number column. Selected text, got the expected selection. Copied text, got the expected copy.
  - Looked at Diffusion. Saw a nice line number column, still. Selected text, got expected selection. Copied text, got expected copy.
  - Looked at Differential. Highlighted stuff, got expected results. Copied stuff, got expected results.

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1123

Differential Revision: https://secure.phabricator.com/D2242
2012-04-16 15:55:16 -07:00
vrana
76b534b560 Don't fetch all commits without blame in Diffusion
Summary:
Otherwise useless query is executed:

  lang=sql
  SELECT c.*
  FROM `repository_commit` c
  ORDER BY c.epoch DESC

Test Plan: /diffusion/X/browse/x

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2186
2012-04-10 09:58:36 -07:00
vrana
347bc357fd Display Browse in Diffusion and Open in Editor links in commit detail
Test Plan:
/rX1
Browse in Diffusion
Open in Editor

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2180
2012-04-10 09:54:38 -07:00
epriestley
a5903d2a53 Use head_key() and last_key() to explicitly communicate intent
Summary:
PHP arrays have an internal "current position" marker. (I think because foreach() wasn't introduced until PHP 4 and there was no way to get rid of it by then?)

A few functions affect the position of the marker, like reset(), end(), each(), next(), and prev(). A few functions read the position of the marker, like each(), next(), prev(), current() and key().

For the most part, no one uses any of this because foreach() is vastly easier and more natural. However, we sometimes want to select the first or last key from an array. Since key() returns the key //at the current position//, and you can't guarantee that no one will introduce some next() calls somewhere, the right way to do this is reset() + key(). This is cumbesome, so we introduced head_key() and last_key() (like head() and last()) in D2161.

Switch all the reset()/end() + key() (or omitted reset() since I was feeling like taking risks + key()) calls to head_key() or last_key().

Test Plan: Verified most of these by visiting the affected pages.

Reviewers: btrahan, vrana, jungejason, Koolvin

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2169
2012-04-09 11:08:59 -07:00
vrana
aa0d0396a6 Highlighting blame is broken if there is an unavailable commit
Test Plan: .../PhotoSnowlift.js?view=blame

Reviewers: jungejason

Reviewed By: jungejason

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2165
2012-04-09 01:14:36 -07:00
epriestley
ee278a302e Improve Diffusion blame views
Summary:
  - Make some effort to simplify the code.
  - Make "Skip Past This Commit" work in Git and Mercurial.
  - Make blame work in Mercurial.
  - Add tooltip hover state to show more information about commits.

Test Plan: Viewed blame views in SVN, Git, Hg. Clicked line numbers, hovered/clicked commits, hovered/clicked "blame past..."

Reviewers: btrahan, vrana, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T378

Differential Revision: https://secure.phabricator.com/D2142
2012-04-07 17:24:35 -07:00
vrana
df67401e24 Add typehints to Diffusion browse file controller
Test Plan: Display commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2139
2012-04-07 16:03:55 -07:00
vrana
5493c0e58e Fix typo 2012-04-07 11:45:31 -07:00
vrana
a234a712cd Disable autoload in search for internal class
Test Plan:
/diffusion/symbol/Exception/?jump=1&type=class&lang=php
/diffusion/symbol/Countable/?jump=1&type=class&lang=php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2124
2012-04-06 12:46:12 -07:00
vrana
1f2028adf0 Render valid HTML
Summary: Also delete some dead code.

Test Plan: /D1

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2120
2012-04-06 09:56:14 -07:00
vrana
d1b7059a2d Open editor from stack trace
Summary:
I've considered that user may have set editor but not checked out Phabricator repositories.
But stack trace is useful mainly for developers.

Test Plan:
Click on path in Unhandled Exception.
Repeat with disabled editor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2107
2012-04-04 18:19:14 -07:00
epriestley
05b4c90bfd Allow Commits to be attached to Tasks using edges
Summary: Use Edges to attach Commits and Tasks. Note, no "edit attached commits" interface from tasks yet since the search backend needs a little work to list commits in a sensible way.

Test Plan: Attached commits to tasks. Looked at commits, looked at tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2105
2012-04-04 17:34:25 -07:00
vrana
582fc847f2 Use assert_instances_of() in Differential
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around Differential.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2103
2012-04-04 15:11:30 -07:00
vrana
7dfdf63948 Fix Jump to HEAD link
Test Plan:
Jump to head.
Go to doctor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2097
2012-04-03 18:39:58 -07:00
vrana
c241f50d77 Use assert_instances_of() in Diffusion
Summary: NOTE: This is not produced by a script so there might be errors. Please review carefully.

Test Plan: Browse around Diffusion.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2094
2012-04-03 16:31:10 -07:00
epriestley
9fc54f4dfb Minor, fix Diffusion home for untracked repositories. 2012-04-03 13:54:10 -07:00
vrana
67e10e60f2 Return $this from setters
Summary:
Most setters returns `$this` but some don't.
I guess it's not by purpose.

Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2085
2012-04-02 18:48:37 -07:00
vrana
c580775d58 Respect view mode on deleted path in Diffusion
Test Plan: /diffusion/X/browse/deleted?view=blame

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2074
2012-04-02 11:16:46 -07:00
epriestley
5945546440 Unify Differential/Maniphest/Diffusion styles and allow commits to be flagged explicitly
Summary:
  - Differential, Maniphest and Diffusion use slightly different styles for the object detail panels.
  - Instead, use the same styles and CSS.
  - Add object actions to Diffusion, including "Flag".

Test Plan: Looked at revisions, tasks and commit. Flagged and unflagged commits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T1041

Differential Revision: https://secure.phabricator.com/D2062
2012-03-30 14:12:10 -07:00
epriestley
b38047006b Show other open revisions affecting the same files in Differential
Summary:
I think this feature is probably good, but Differential is also really starting to get a lot of stuff which is not the diff in it. Not sure how best to deal with that.

The mixed table styles are also pretty ugly.

So I guess this is more feedback / proof-of-concept, I think I want to try to improve it somehow before I land it.

Test Plan: Looked at some diffs, some had an awkward, ugly list of diffs affecting the same files.

Reviewers: bill, aran, btrahan

Reviewed By: aran

CC: aran, epriestley

Maniphest Tasks: T829

Differential Revision: https://secure.phabricator.com/D2027
2012-03-30 10:13:08 -07:00
epriestley
36ab0c3313 Fix local-clobbering iterators in phabricator/
Summary:
These are the issues identified by the linter in D2052. I don't think any cause bugs, but they are all reasonable errors to raise and the linter correctly
detected that they are suspicious.

Test Plan: Mostly inspection.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2053
2012-03-29 13:24:06 -07:00
Nick Harper
8acd596925 Set full path for DiffusionRepositoryPath in svn diffusion browse query
Summary:
The full path field of the DiffusionRepositoryPath object is used by the
DiffusionBrowseController when viewing a directory with a readme file, so
we should set this field.

Test Plan: loaded a directory containing a readme in a svn repo

Reviewers: epriestley, jungejason, emiraga

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2045
2012-03-28 20:51:41 -07:00
epriestley
ea148c4841 In the SVN "Change" view, identify the last time a file was modified instead of 404ing
Summary:
If a file was last modified at revision 50 and you look at revision 55, we currently 404. Instead, always identify the last modification.

Also simplify some of the query objects.

Test Plan: Viewed after-modification revisions for several files in SVN.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley

Maniphest Tasks: T851

Differential Revision: https://secure.phabricator.com/D2028
2012-03-26 21:58:02 -07:00
epriestley
3bacba7e9f Show parent commits in Diffusion
Summary: Show parent commit information to make it easier to understand merges.

Test Plan: Looked at commits in SVN, hg, git.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T961

Differential Revision: https://secure.phabricator.com/D2021
2012-03-26 12:21:48 -07:00
epriestley
59b49e6429 In Diffusion browse views, show a README file if one exists
Summary: COMPLETELY ORIGINAL IDEA

Test Plan: Browsed around Phabricator, got helpful readmes in some cases.

Reviewers: davidreuss, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2022
2012-03-26 12:21:39 -07:00
epriestley
0f94b49b33 Minor, don't collapse comment content if there are inlines.
Auditors: vrana
2012-03-26 10:05:15 -07:00
epriestley
2044e51206 Add "Resign from Audit" and "Close Audit" actions to Diffusion
Summary:
See some discussion in D2002. Add two new actions:

  - Resign: (auditor only) closes your open request (user request ONLY) by putting it in a "resigned" state.
  - Close: (author only) closes all open requests by putting them in a "closed" state.

@davidreuss, this is probably conflict-city with D2002 -- I'll wait for you to land first and then handle the merge on my end.

Test Plan: Resigned from and closed audits.

Reviewers: 20after4, davidreuss, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D2013
2012-03-26 09:44:06 -07:00
epriestley
7e519e026a Minor, part 2/2, fix class casing. 2012-03-24 12:52:14 -07:00
epriestley
4012ae1cd3 Minor, part 1/2, fix class casing. 2012-03-24 12:11:51 -07:00
epriestley
c2f50e258a Render pretty graphical traces for commit branches, etc
Summary: I AM A WIZARD

Test Plan: BEHOLD

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T961

Differential Revision: https://secure.phabricator.com/D2007
2012-03-23 17:11:15 -07:00
Bill Fumerola
bee69f9ce2 do not re-use output when an exception occurs
Summary:
$stdout from the previous run would be reused if an exception
occurred

Test Plan: that's a negative, ghostrider.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2008
2012-03-23 16:29:03 -07:00
epriestley
d28eb759d6 Show merged changes in commit views for merges
Summary:
When a commit is a merge, show what it merged.

Also fix some bugs:

  - Mercurial queries may contain ":", but mercurial rev ranges may also contain ":". A rev range with a branch that has a ":" in it is ambigiuous, e.g. branch "a:b" might appear in a rev range like "a🅱️0", which can not be parsed. Use stable commit names instead.
  - Mercurial stable commit name implementation was broken, fix it.
  - Extend DiffusionHistoryQuery from DiffusionQuery to share code.
  - Fix a bug where Mercurial's main browse list would not show the most recent commit if it was a merge commit.

Test Plan: Generated a bunch of mercurial/git merge commits and looked at them, they seemed to accurately represent the repository state.

Reviewers: btrahan, Makinde

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T961

Differential Revision: https://secure.phabricator.com/D2005
2012-03-23 15:32:26 -07:00
vrana
1eca595e4d Fix Change link in Diffusion history
Summary:
And maybe also something else.
And hopefully don't break anything.

Test Plan: /diffusion/X/history/..., click on //Modified//.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1996
2012-03-22 15:53:01 -07:00
epriestley
620e936cba Fix symbol URI generation to include default branch name for relevant repositories
Summary: We need to build a request in order to pick up an appropriate default branch name, instead of using the raw static generator.

Test Plan: Clicked a symbol link, got /master/path/blahblah

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1982
2012-03-21 16:58:44 -07:00
vrana
74cc558ed8 Rename column Size to Commits in Browser Repositories table
Test Plan: /diffusion/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1981
2012-03-21 14:54:17 -07:00
epriestley
c0aac8267d Improve Diffusion behavior for externals
Summary:
  - Feature request from Airtime that I missed in the feedback notes, came up yesterday.
  - Identify git submodules as "FILE_SUBMODULE", not "FILE_NORMAL".
  - Link git submodules to an external resolver endpoint, which tries to find commits in tracked repositories.
  - Identify git symlinks as "FILE_SYMLINK", not "FILE_NORMAL".
  - Add folder, file, symlink and externals icons.

Test Plan:
  - externals/javelin is now identified as a submoudule and links to Javelin, not identified as a file and links to error.
  - bin/phd is now identified as a symlink.
  - Interfaces have pretty icons.

Reviewers: btrahan, cpiro, ddfisher, keebuhm, allenjohnashton

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1975
2012-03-21 14:01:20 -07:00
vrana
5c5ead2666 Fix links after D1921
Test Plan:
Search for some symbol. Click on the result. Verify that there is not // in URL.
Click on the link from generated exception.
View history in Diffusion, click on Browse.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1979
2012-03-21 13:33:57 -07:00
vrana
ccc258fa85 Jump to PHP Manual for PHP's internal classes and interfaces in symbol search
Test Plan: Clicked on `Exception` in `throw new Exception`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1978
2012-03-21 13:31:22 -07:00
vrana
12973fca36 Fix a typo in exception message 2012-03-21 12:26:46 -07:00
vrana
f6fcf034c1 Ensure displaying PHP Manual function page for PHP function symbols
Summary:
Some PHP functions have the same name as extension or class.
php.net prefers them over functions.

Test Plan:
http://www.php.net/hash
http://www.php.net/function.hash
http://www.php.net/function.hash_final

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1977
2012-03-21 10:49:33 -07:00
epriestley
1227c8d5da Add "View Options" dropdown to Differential / Audit
Summary:
Depends on D1921. Depends on D1899.

Add the "View Options" dropdown menu to Diffusion, with options like "show standalone", "show raw file", "show all", etc.

Test Plan: Viewed commits in Differential and Diffusion.

Reviewers: davidreuss, nh, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1932
2012-03-19 19:57:41 -07:00
epriestley
e42c29f4ec Add inline comments to the web view of Diffusion / Audit
Summary: Depends on D1928. Uses the new UI element to display inlines in Diffusion.

Test Plan: Looked at a commit with inline comments, saw them in the summaries.

Reviewers: davidreuss, nh, btrahan

Reviewed By: davidreuss

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1929
2012-03-19 19:56:06 -07:00
epriestley
0a4cbdff5e Straighten out Diffusion file integration
Summary:
This is in preparation for getting the "View Options" dropdown working on audits.

  - Use Files to serve raw data so we get all the security benefits of the alternate file domain. Although the difficulty of exploiting this is high (you need commit access to the repo) there's no reason to leave it dangling.
  - Add a "contentHash" to Files so we can lookup files by content rather than adding some weird linker table. We can do other things with this later, potentially.
  - Don't use 'data' URIs since they're crazy and we can just link to the file URI.
  - When showing a binary file or an image, don't give options like "show highlighted text with blame" or "edit in external editor" since they don't make any sense.
  - Use the existing infrastructure to figure out if things are images or binaries instead of an ad-hoc thing in this class.

Test Plan: Looked at text, image and binary files in Diffusion. Verified we reuse existing files if we've already generated them.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1899
2012-03-19 19:52:24 -07:00
epriestley
30ae22bfcf Fix many encoding and architecture problems in Diffusion request and URI handling
Summary:
Diffusion request/uri handling is currently a big, hastily ported mess. In particular, it has:

  - Tons and tons of duplicated code.
  - Bugs with handling unusual branch and file names.
  - An excessively large (and yet insufficiently expressive) API on DiffusionRequest, including a nonsensical concrete base class.
  - Other tools were doing hacky things like passing ":" branch names.

This diff attempts to fix these issues.

  - Make the base class abstract (it was concrete ONLY for "/diffusion/").
  - Move all URI generation to DiffusionRequest. Make the core static. Add unit tests.
  - Delete the 300 copies of URI generation code throughout Diffusion.
  - Move all URI parsing to DiffusionRequest. Make the core static. Add unit tests.
  - Add an appropriate static initializer for other callers.
  - Convert all code calling `newFromAphrontRequestDictionary` outside of Diffusion to the new `newFromDictionary` API.
  - Refactor static initializers to be sensibly-sized.
  - Refactor derived DiffusionRequest classes to remove duplicated code.
  - Properly encode branch names (fixes branches with "/", see <https://github.com/facebook/phabricator/issues/100>).
  - Properly encode path names (fixes issues in D1742).
  - Properly escape delimiter characters ";" and "$" in path names so files like "$100" are not interpreted as "line 100".
  - Fix a couple warnings.
  - Fix a couple lint issues.
  - Fix a bug where we would not parse filenames with spaces in them correctly in the Git browse query.
  - Fix a bug where Git change queries would fail unnecessarily.
  - Provide or improve some documentation.

This thing is pretty gigantic but also kind of hard to split up. If it's unreasonably difficult to review, let me know and I can take a stab at it though.

This supplants D1742.

Test Plan:
  - Used home, repository, branch, browse, change, history, diff (ajax), lastmodified (ajax) views of Diffusion.
  - Used Owners typeaheads and search.
  - Used diffusion.getrecentcommitsbypath method.
  - Pushed a change to an absurdly-named file on an absurdly-named branch, everything worked properly.

{F9185}

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1921
2012-03-19 19:52:14 -07:00
epriestley
0f45e85ce5 Silence Mercurial warning for lastmodified query; minor cleanup
Summary:
See <https://github.com/facebook/phabricator/issues/102>. Commit data may not be available for unpared commits, but we'll raise a warning about $commit_data in that case (the UI correctly handles missing $commit_data).

Also some minor cleanup / UI fixes.

Test Plan: Browsed around hg / git repos, including unparsed commits.

Reviewers: btrahan, killermonk

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1961
2012-03-19 19:19:55 -07:00
epriestley
3db02a584c Fix some warnings about markup engines in Audit/Diffusion inline comments
Summary:
Split from D1921.

  - DifferentialChangesetParser doesn't have this property declared.
  - We weren't providing a markup engine, which caused some warnings.
  - This would cause failures if comment caches weren't present. Currently, they always will be though unless someone has wiped them explicitly in the DB.

Test Plan: Viewed a diff with inline comments, didn't get any warnings in the log.

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1925
2012-03-19 19:17:59 -07:00
epriestley
8c141fdfd1 Fix an issue where DiffusionGitBrowseQuery incorrectly parses filenames with spaces
Summary:
Split from D1921. We'll explode each line into too many parts currently, if the filename contains spaces.

Also use -z to get \0 newlines.

Test Plan: Browsed a directory containing files with spaces in their names, links etc were correct.

Reviewers: nh, vrana, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1924
2012-03-19 19:17:50 -07:00
epriestley
81fc0b8843 Add keyboard navigation to Diffusion / Audit
Summary: This diff is really complicated, only a master programmer like myself could have accomplished it.

Test Plan: derrrrrp

Reviewers: davidreuss, nh, btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1936
2012-03-18 19:44:28 -07:00
vrana
af260c38cb Display time of revision in blame
Test Plan: Hover revision in blame

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1907
2012-03-14 21:31:16 -07:00
epriestley
900190b2fe Add inline comments to Diffusion/Audit
Summary:
  - Add inline comments to Audits, like Differential.
  - Creates new storage for the comments in the Audits database.
  - Creates a new PhabricatorAuditInlineComment class, similar to DifferentialInlineComment.
  - Defines an Interface which Differential and Audit comments conform to.
  - Makes consumers of DifferentialInlineComments consume objects which implement that interface instead.
  - Adds save

NOTE: Some features are still missing! Wanted to cut this off before it got crazy:

  - Inline comments aren't shown in the main comment list.
  - Inline comments aren't shown in the emails.
  - Inline comments aren't previewed.

I'll followup with those but this was getting pretty big.

@vrana, does the SQL change look correct?

Test Plan:
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Diffusion, on the left and right side of diffs.
  - Created, edited, deleted, replied to, reloaded and saved inline comments in Differentila, on the left and right side of primary and diff-versus-diff diffs.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1898
2012-03-14 12:56:01 -07:00
vrana
e0bd67f77b Unmisplace edit button in Diffusion
Summary:
It is misplaced from the beginning but it got worse after some CSS tweaks.

I was also thinking about using the same DropdownMenu as in Differential but I don't feel strongly about it to do it myself.

Test Plan:
Display file in Diffusion.
Repeat with disabled Editor.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1892
2012-03-14 09:57:57 -07:00
Nick Harper
7c9057854b [svn1.7] Fix thrown exception when browsing diffusion with added directories
Summary: svn cat returns a non-zero exit status when trying to cat a directory.

Test Plan: Browsed diffusion in my sandbox

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1882
2012-03-13 16:11:28 -07:00
epriestley
11cccb98c2 Add "final" to more classes
Summary: No big surprises here, delted the unused "DarkConsole" class.

Test Plan: Ran 'testEverythingImplemented' to verify I wasn't finalizing anything we extend.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1876
2012-03-13 11:18:11 -07:00
epriestley
f158b32a54 Minor, formalize changeset response class. 2012-03-12 21:39:05 -07:00
epriestley
4c147b496c Minor, unbreak other changeset APIs. 2012-03-12 21:02:15 -07:00
vrana
d5bf30bb48 Prepare database for UTF-8
Summary: D1830#8

Test Plan:
`scripts/sql/upgrade_schema.php`
Try adding duplicate SSH Public Key - failed.
Try adding new SSH Public Key - succeeded.

Reviewers: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1852
2012-03-09 18:56:22 -08:00
epriestley
b2890eeb0e Add "final" to all Phabricator "Controller" classes
Summary:
These are all unambiguously unextensible. Issues I hit:

  - Maniphest Change/Diff controllers, just consolidated them.
  - Some search controllers incorrectly extend from "Search" but should extend from "SearchBase". This has no runtime effects.
  - D1836 introduced a closure, which we don't handle correctly (somewhat on purpose; we target PHP 5.2). See T962.

Test Plan: Ran "testEverythingImplemented" unit test to identify classes extending from `final` classes. Resolved issues.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T795

Differential Revision: https://secure.phabricator.com/D1843
2012-03-09 15:46:25 -08:00
epriestley
1c80a4eb58 Add a "properties" table to the Repository view in Diffusion
Summary: Notably, expose the clone URI / remote URI, after stripping credentials.

Test Plan: Looked at a repository.

Reviewers: btrahan, nh

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T91

Differential Revision: https://secure.phabricator.com/D1811
2012-03-08 12:46:19 -08:00
vrana
afe38572fe Display blame colors in multiline highlighting
Summary:
Multiline highlighting didn't work well together with blame.

Blame Rev: rPe24a6ac

Test Plan: /diffusion/...$596-598?view=blame

Reviewers: epriestley

Reviewed By: epriestley

CC: Koolvin, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1814
2012-03-07 13:49:38 -08:00
vrana
4c8f405fcc Use wide links only for line and blame-prev in Diffusion
Summary: D1701 disallowed me selecting authors and revisions by mouse, grrr.

Test Plan: View file, hover over <th>, click.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1736
2012-02-29 18:22:04 -08:00
vrana
040f934adf Allow blaming of empty file in Diffusion
Summary: Blame of empty call currently throws AphrontQueryParameterException.

Test Plan: Blame empty file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1734
2012-02-29 16:32:34 -08:00
epriestley
1eeaeb62e4 Remove commit list from Diffusion in favor of Audit commit list
Summary:
We can drive this query better from the Audit tool now; get rid of the Diffusion
version.

Preserve usernames in URIs as per T900.

Test Plan: Clicked "Commits" from profile. Browsed audit commit filters.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1713
2012-02-28 21:12:08 -08:00
epriestley
8a0a00f118 Make PhabricatorRepositoryCommmit schema changes for audit
Summary:
  - Add a proper mailKey field to make these things mailable. Backfill all
existing objects.
  - Denormalize authorPHID to the commit object so we can query by it
efficiently in a future diff. We currently use the search engine to drive
"commits by author" but that's not so good for audit, which needs more
constraints.
  - Add an overall audit status field so we can efficiently query "commits that
needs your attention".
  - Add enough code to convince myself that these fields are basically
reasonable and work correctly.

Test Plan:
  - Ran schema upgrades. Checked database state afterward.
  - Ran "reparse.php --owners --herald" to verify worker changes.
  - Looked at a commit, altered aggregate status via audits / reparse.php,
verified it responded correctly.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley, nh

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1706
2012-02-28 21:06:34 -08:00
epriestley
c7094d2def Add preview and drafts to audits
Summary: Add comment previews and saved drafts to audits, like Maniphest /
Differential.

Test Plan: Typed stuff into the box. Got a preview. Reloaded page. Stuff was
still there. Submitted comment. Stuff is gone.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1699
2012-02-27 13:00:23 -08:00
epriestley
1094527072 Allow Herald to trigger audits for users or projects
Summary:
Allows you to write a commit rule that triggers an audit by a user (personal
rules) or a project (global rules).

Mostly this is trying to make auditing more lightweight and accessible in
environments where setting up Owners packages doesn't make sense.

For instance, Disqus wants a rule like "trigger an audit for everything that
didn't have a Differential revision". While not necessarily scalable, this is a
perfectly reasonable rule for a small company, but a lot of work to implement
with Owners (and you'll get a lot of collateral damage if you don't make every
committer a project owner).

Instead, they can create a project called 'Unreviewed Commits' and write a rule
like:

	- When: Differential revision does not exist
 	- Action: Trigger an Audit for project: "Unreviewed Commits"

Then whoever cares can join that project and they'll see those audits in their
queue, and when they approve/raise on commits their actions will affect the
project audit.

Similarly, if I want to look at all commits that match some other rule (say,
XSS) but only want to do it like once a month, I can just set up an audit rule
and go through the queue when I feel like it.

NOTE: This abuses the 'packagePHID' field to also store user and project PHIDs.
Through the magic of handles, this (apparently) works fine for now; I'll do a
big schema patch soon but have several other edits I want to make at the same
time.

Also:

	- Adds an "active" fiew for /audit/, eventually this will be like the
Differential "active" view (stuff that is relevant to you right now).
	- On commits, highlight triggered audits you are responsible for.

Test Plan: Added personal and global audit triggers to Herald, reparsed some
commits with --herald, got audits. Browsed all audit interfaces to make sure
nothing exploded. Viewed a commit where I was responsible for only some audits.
Performed audits and made sure the triggers I am supposed to be responsible for
updated properly.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1690
2012-02-27 09:36:30 -08:00
vrana
991fee2118 Use wide links instead of fake cursor in Differential
Summary:
Current approach has several problems:

- if there is no link in the cell then it still shows a link cursor
- if there is a link then it is clickable only on the text

Test Plan:
Display file in Differential, hover over cell with link.
Repeat for Paste.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1701
2012-02-26 13:12:07 -08:00
Korvin Szanto
e24a6acf58 Multiline Highlighting in Diffusion
Summary:
I added multiline highlighting with the syntax:

  http://site/path/to/file$from-to

NOTE: you can reverse the from and to

Test Plan: Open a file in diffusion and attempt to highlight multiple lines

Reviewers: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1693
2012-02-25 12:32:59 -08:00
epriestley
e5f3ad14e1 Allow audit comments to be added from Diffusion
Summary:
This is intended to supplant the existing "audit edit" interface. I've changed
them to both drive down the same write pathway, but the UIs are still different.
I'll fully merge them in a future diff.

Add a comment box (like Maniphest and Differential) to Diffusion. When users
make comments, their comments appear on the commit. Any audits triggers they are
responsible for are updated to reflect actions they take, as well.

Currently, audits can only be triggered by packages, but I intend to allow them
to be triggered by users and projects (via herald rules) in an upcoming diff.
Thus some of the language like "projects, users or packages" when the code is
clearly dealing only with "packagePHID".

Test Plan: Made audit updates via commit interface and via existing edit
interface. Verified both interfaces updated correctly, and that audit
responsibility rules were applied properly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1688
2012-02-24 15:04:53 -08:00
epriestley
5f46a61e6d Show audit comments on the Diffusion commit view
Summary: We already allow you to create comments, but we don't show them on the
commit page. After style / view unification this is easy; show comments on the
commit page.

Test Plan: Made comments on a commit using the audit too, saw them show up in
Diffusion.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1687
2012-02-24 14:14:39 -08:00
epriestley
97ea6ea619 Add a basic first-class audit UI
Summary:
Currently, audits are only accessible through the Owners tool. Start moving them
to their own first-class tool in preparation for broader audit integration.

  - Lay some infrastructure groundwork (e.g. AuditQuery).
  - Build a basic /audit/ view.
  - Show audits on the commit page in Diffusion.

This has some code duplication with stuff we've already got, but I'll merge
everything together as we move forward on this.

Test Plan: Looked at /audit/ and a commit.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

Differential Revision: https://secure.phabricator.com/D1685
2012-02-24 13:02:14 -08:00
Nick Harper
07e5591015 [diffusion] Fix pending differential revisions list
Summary:
DifferentialRevisionListView requires setFields to be called before
calling getRequiredHandlePHIDs; this adds that call for DiffusionController

Test Plan:
loaded diffusion and saw the "Pending Differential Revisions" section
populated, and no errors in the darkconsole

Reviewers: epriestley, btrahan, jungejason

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1665
2012-02-21 22:47:36 -08:00
epriestley
cd651001b6 Add a contextual "scope" dropdown for searches
Summary: Add a "Search for ... in (document group)" thing that picks the current
scope based on the current application.

Test Plan: Conducted searches in several browsers.

Reviewers: btrahan, skrul

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T858

Differential Revision: https://secure.phabricator.com/D1610
2012-02-14 17:00:12 -08:00
vrana
7a0337054b Add Owners search to Diffusion
Test Plan:
/diffusion/X/browse/:/file
Click Search Owners.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1572
2012-02-04 10:23:36 -08:00
vrana
82c6f275bd Add links to empty result in Diffusion
Summary: I think that these are the only links that are useful - commit which
deleted the path and last version of the path.

Test Plan: Display deleted path, click on links.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1536
2012-02-01 11:57:48 -08:00
vrana
073b80c505 Add title="" for Blame previous revision glyph
Summary: Adds 7th parameter :-(.

Test Plan: Display blame.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1509
2012-01-27 14:09:10 -08:00
vrana
067c7f8a74 Display links to editor in Differential and Diffusion
Summary:
It is possible to open a file in editor by registering a custom URI scheme
(pseudo-protocol). Some editors register it by default.
Having links to open the file in external editor is productivity booster
although it is a little bit harder to set up.
There are several other tools using file_link_format configuration directive
(XDebug, Symfony) to bind to this protocol.
I've added the example with editor: protocol which can be used as a proxy to
actual editor (used by Nette Framework:
http://wiki.nette.org/en/howto-editor-link).

Test Plan:
Configure Editor Link in User Preferences.
Register URI scheme in OS.
Open a file in Diffusion. Click on the Edit button.
Open a revision in Differential. Click on the Edit button.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1422
2012-01-24 10:42:33 -08:00
vrana
854ed4ea53 Display link in commit ToC for directories
Summary:
They are present in the document so there is no reason to omit the links to
them.
Similar to D1412.

Test Plan: Display commit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1464
2012-01-23 13:23:17 -08:00
vrana
e142c7e26c Improve <title> on Diffusion pages
Test Plan:
Visit /rX123.
Visit /diffusion/X/.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1463
2012-01-21 01:33:41 -08:00
epriestley
8b4b614d15 Show all branches a commit appears on
Summary:
We show the contextual branch (always the repository default branch) when
viewing a commit. Instead, show all branches the commit appears on.

Also pull some of the duplicated DiffusionXQuery stuff into a DiffusionQuery
base class, I'll do a followup to reduce more duplication.

Test Plan: Looked at a commit in Git. My HG and SVN setups are a little borked
so I kind of faked tests in them -- I'm fixing them now.

Reviewers: btrahan, jungejason, fratrik

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T768

Differential Revision: https://secure.phabricator.com/D1458
2012-01-19 21:12:44 -08:00
vrana
c0592b55d7 Use generic URIs
Summary:
/diffusion/X/history/?copies=0 is same as /diffusion/X/history/
/countdown/1/?chrome=1 is same as /countdown/1/

Test Plan:
Visit /diffusion/X/history/, click on Show/Hide Copies/Branches twice.
Visit /countdown/1/, click on Disable/Enable Chrome twice.

Reviewers: epriestley, tuomaspelkonen

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1424
2012-01-16 14:53:05 -08:00
epriestley
1903bb80bb Add a link from Differential to Diffusion
Summary:
Provide an easy way to jump to Diffusion from Differential if we have
the data we need to connect them.

Test Plan: Tested menu in linked and unlinked diffs. Used menu item.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley, btrahan

Maniphest Tasks: T309

Differential Revision: https://secure.phabricator.com/D1326
2012-01-05 18:03:08 -08:00
epriestley
99231ebba4 Allow Git repositories to track only some branches
Summary:
Some installs use Git as the backbone of a CI framework or use a Git remote to
share patches. The tracker scripts currently recognize associated revisions as
"Committed" when they appear in any branch, even if that branch is
"alincoln-personal-development_test_hack" or whatever.

To address the broadest need here, allow Git repositories to be configured to
track only certain branches instead of all branches.

This doesn't allow you to import a branch into Diffusion but ignore it in
Differential. Supporting that is somewhat technically complicated because the
parser currently goes like this:

  - Look at HEAD of all branches.
  - For any commits we haven't seen before, follow them back to something we
have seen (or the root).
  - "Discover" everything new.

Since this doesn't track <branch, commit> pairs, we currently don't have enough
information to tell when a commit appears in a branch for the first time, so we
don't have anywhere we can put a test for whether that branch is tracked and do
the Differential hook only if it is.

However, I think this cruder patch satisfies most of the need and is simple and
obvious in its implementation.

See also D1263.

Test Plan:
  - Updated a Git repository with various filters: "", "master, remote", "derp",
"  ,,, master   ,,,,,"
  - Edited SVN and Mercurial repositories to verify they didn't get caught in
the crossfire.
  - Ran daemon in debug mode on libphutil with filter "derp", got exception
about no tracked branches. Ran with filter "master", got tracking. Ran with no
filter, got tracking.
  - Looked at Diffusion with "derp" and "master", saw no branches and "master"
respectively.
  - Added unit tests to cover filtering logic.

Reviewers: btrahan, jungejason, nh, fratrik

Reviewed By: fratrik

CC: aran, fratrik, epriestley

Maniphest Tasks: T270

Differential Revision: https://secure.phabricator.com/D1290
2012-01-04 10:20:34 -08:00
epriestley
efb0fa739f Make tracked git repositories use an implicit 'origin' remote
Summary:
See T624. I originally wrote this to require an explicit remote, but this
creates an ugly "origin:" in all the URIs and makes T270 more difficult.

Treat all branch names as implying 'origin/'.

Test Plan:
  - Pulled and imported a fresh copy of libphutil without issues.
  - Browsed various git repositories.
  - Browsed Javelin's various branches.
  - Ran upgrade script, got a bunch of clean 'origin/master' -> 'master'
conversions.
  - Tried to specify an explicit remote in a default branch name.
  - Unit tests.

Reviewers: nh, jungejason, btrahan

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T624

Differential Revision: https://secure.phabricator.com/D1269
2011-12-29 08:35:32 -08:00
epriestley
99a9c082f9 Fix a fatal on the commit author list
Summary: I think we only hit this because I mucked around with the database to
recover from the runaway parse of the Diviner repository (now prevented by
D1253), but be more robust against missing data in this interface.

Test Plan: After applying this patch, no longer received a fatal on the commit
history page for users linked to nonexistant/bogus commits.

Reviewers: jack, btrahan, jungejason, aran

Reviewed By: aran

CC: aran

Maniphest Tasks: T701

Differential Revision: https://secure.phabricator.com/D1264
2011-12-22 07:10:34 -08:00
epriestley
b258095124 Expose symbol information over Conduit
Summary: I want to add a command like "where is ArcanistUnitTestEngine" to
phabot. I also want to add a symbol typeahead to Diffusion and generally finish
up that feature since it's useful but only half-implemented. Consolidate the
query logic and expose the data over Conduit.

Test Plan: Used /symbol/ and Conduit to lookup symbols.

Reviewers: btrahan, jungejason

Reviewed By: jungejason

CC: aran, jungejason

Maniphest Tasks: T315

Differential Revision: https://secure.phabricator.com/D1260
2011-12-22 06:44:55 -08:00
epriestley
e0a56cb938 Clean up two more sha1 instances
Summary: See T547. One of these I just missed in D1000; the comment change just
makes it easier to audit use of hash functions by cleaning up "grep" output.

Test Plan: Ran isolation unit test.

Reviewers: btrahan, jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 1124
2011-11-20 14:21:26 -08:00
Bob Trahan
5c21b5345d execx ==> execxLocalCommand for git libraries in diffusion
Summary:
this was fairly mechanical at the end of the day

note that future/exec got removed by the code generation robots
post this change

Test Plan:
clicked around diffusion a bunch looking for errors.

For a given repo (say http://phabricator.dev/diffusion/BOBALIE/)
 - http://phabricator.dev/diffusion/BOBALIE/history/
 - http://phabricator.dev/diffusion/BOBALIE/history/origin:master/.arcconfig
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/
 - http://phabricator.dev/diffusion/BOBALIE/browse/origin:master/.arcconfig
 - http://phabricator.dev/rBOBALIEbfede2e8ea9435644968e2e76c0bac8949fb7d06

For a given file (say
http://phabricator.dev/diffusion/BOBALIE/change/origin:master/.arcconfig;bfede2e8ea9435644968e2e76c0bac8949fb7d06)
 - history view*
 - browse view
 - change view

* found a bug where the history view doesn't have the change view in the left
hand UI
will fix laters

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1095
2011-11-09 16:21:59 -08:00
David Reuss
4e900c096f Convert "falsey" binary hunks if we have a repository encoding
Summary:
This adds an encoding detail to the repository, so we can attempt to
convert hunks previously detected as binary.

We also add the encoding information to the arcanist projectinfo
API so we can pull the information if we have it when uploading changes
via arc.

Test Plan:
Changed encoding through the edit UI, and saw "This is binary file", and
changed it back and saw the correct output from the diff.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: 1009
2011-10-28 08:04:57 -07:00
Jason Ge
13eee1a344 Add test to check all symbols can be loaded
Summary:
make sure all symboles can be loaded to avoid issues like missing
methods in descendants of abstract base class.

Test Plan:
ran it and verified it passes; remove a method in a descendant class
and verified that the test failed.

Reviewers: epriestley, nh

Reviewed By: nh

CC: aran, nh, jungejason

Differential Revision: 1023
2011-10-20 16:43:13 -07:00
tuomaspelkonen
b63393d056 Remove the <a> tags from author name in 'View as Plain Text with Blame'
Summary: It looked stupid.

Test Plan: It looks better now and other options still work.

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, tuomaspelkonen

Differential Revision: 1017
2011-10-19 15:28:43 -07:00
epriestley
78689df4d4 Fix missing branch component in symbol crossreference URIs. 2011-10-09 18:36:00 -07:00
epriestley
254f606e89 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
2011-10-09 17:58:17 -07:00
epriestley
8ce5dd31f6 Show open Differential revisions in Diffusion browse views
Summary:
Still some rough edges, but this adds a table of open revisions to Diffusion.
See T262.

I'll make this a little better (e.g., "see all.." instead of arbitrary 10 cap,
or maybe move to top-level nav?) but I think I have to refactor some other stuff
first. This should let us root out any major issues, at least.

NOTE: You must associate Arcanist Projects with Repositories (in Repositories ->
Arcanist Projects -> Edit) for this to work!

Also made paths include all parent paths so that browse views of directories
will work.

Test Plan: Uploaded a diff which affected "/blah", it appeared when browsing "/"
and "/blah".

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 979
2011-10-06 10:27:54 -07:00
epriestley
8e8d91a1ff Allow Diffusion to display the initial commit in Git repositories
Summary: See T507. Since you can't do "xxxxxxxx^" where "xxxxxxxx" is the first
commit in a repository, fall back to diffing against the empty tree if we fail
to diff against the parent commit.

Test Plan: Looked at the first commit in libphutil on my local.

Reviewers: edward, jungejason, nh, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, edward, epriestley, nh

Differential Revision: 953
2011-09-30 11:56:19 -07:00
epriestley
b1e1b1f9bd Basic support for Mercurial in Diffusion
Summary: Change import script plus almost all the view stuff. Still some rough
edges but this seems to mostly work. Blame is currently unsupported but I think
everything else works properly.

Test Plan:
Imported the hg repository itself. It doesn't immediately seem completely
broken. Here are some screens:

https://secure.phabricator.com/file/view/PHID-FILE-1438b71cc7c4a2eb4569/
https://secure.phabricator.com/file/view/PHID-FILE-3cec4f72f39e7de2d041/
https://secure.phabricator.com/file/view/PHID-FILE-2ea4883f160e8e5098f9/
https://secure.phabricator.com/file/view/PHID-FILE-35f751a36ebf65399ade/

All the parsers were able to churn through it without errors.

Ran the new "reparse.php" script in various one-commit and repository modes.

Browsed/imported some git repos for good measure.

NOTE: The hg repository is only 15,000 commits and around 1,000 files.
Performance is okay but hg doesn't provide performant, native APIs to get some
data efficiently so we have to do some dumb stuff. If some of these interfaces
are cripplingly slow or whatever, let me know and we can start bundling some
Mercurial extensions with Arcanist.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde, epriestley

Differential Revision: 960
2011-09-27 19:28:57 -07:00
epriestley
9155369668 Add a helper function to DiffusionPathIDQuery
Summary:
Just breaking D960 into some smaller parts, this is a standalone method used in
Mercurial parsing.

(There's a bad version of this function in the SVN stuff but I'll get rid of it
the next time I'm in there.)

Test Plan: See D960.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, Makinde

Differential Revision: 965
2011-09-27 11:05:12 -07:00
epriestley
1c1f749eba Add an "arcanist.projectinfo" Conduit call
Summary:
We currently rely on "remote_hooks_enabled" in .arcconfig to determine whether
commands like "arc amend" and "arc merge" should imply "arc mark-committed".

However, this is a historical artifact that is now bad for a bunch of reasons:

  - The option name is confusing, it really means 'repository is tracked'.
  - The option is hard to discover and generally sucks.
  - We can empirically determine the right answer since we now know if a project
is in a tracked repository.

Add a call which arcanist can make on these workflows to figure out if it is
interacting with a project in a tracked repository or not.

Also added an "isTracked()" convenience method to reduce the number of magic
strings all over the place.

Test Plan: Ran "arcanist.projectinfo" for nonexistent, untracked and tracked
projects.

Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran

Reviewed By: Makinde

CC: aran, epriestley, Makinde

Differential Revision: 945
2011-09-21 14:19:14 -07:00
epriestley
b64f252f8b Fix a dirname() edge case in Diffusion
Summary:
dirname('x') returns '.', not '/'; this caused some issues for repositories with
files at the root.

There are some cases in the parsers where I should probably swap this out too
but I'll wait until I'm doing some more rigorous testing since that stuff is a
bit fragile and this fixes an immediate issue.

Test Plan: Ran unit tests. Viewed a file at root level in a test repository.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: nh

CC: aran, nh

Differential Revision: 932
2011-09-15 07:45:15 -07:00
epriestley
43a3f4d234 Build an "affected path" index when attaching diffs to revisions
Summary: See T262. This creates the index on the Differential side which we need in order to execute this query efficiently on the Diffusion side.

Also renames "DiffusionGitPathIDQuery" to "DiffusionPathIDQuery", this query object has nothing to do with git.

Test Plan: Attached top-level and sub-level diffs to revisions and verified they populated the table with sensible data.

Reviewers: bmaurer, aravindn, fmoo, jungejason, nh, tuomaspelkonen, aran

CC:

Differential Revision: 931
2011-09-15 07:45:14 -07:00
epriestley
888af7309a Add a simple symbol lookup interface for cross-references
Summary: This will get fancier, but here's a basic interface for doing symbol
lookups. Still all pretty tentative.

Test Plan: Looked up various things, got some sensible results.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: tuomaspelkonen

CC: aran, tuomaspelkonen

Differential Revision: 900
2011-09-13 08:49:45 -07:00
Jaap Weel
bd778b4c8e Allow Diffusion to display PDF files
Summary:
When Diffusion encounters an image file, it displays it as an
image, but when it encounters a PDF file, it currently shows only some
gibberish. This fixes that.

Test Plan:
I tried it. Embedding a large PDF in a data URL is a little
bit slow, but it works.

Reviewers: tuomaspelkonen, epriestley, gc3, waltermundt, jungejason, nh

Reviewed By: epriestley

CC: aran, tuomaspelkonen, epriestley, jaapweel

Differential Revision: 915
2011-09-09 13:14:49 -07:00
Abdul Qabiz
6355b291ed - Added getRemoteCommandFuture(..) and getLocalCommand Future(..) methods to PhabricatorRepository
- Removed irrelevant csprintf(..)
  - Updated code to use $repository->getRemoteURI()
  - Updated code to use getRemoteCommandFuture(..) in Diffusion code
  - Updated code to use $repository->getRemoteURI()
2011-09-09 01:16:48 +05:30
epriestley
8f3b342287 Improve several Diffusion UI error states
Summary:
Give users better errors and UI:

  - For subpath SVN repositories, default the path to the subdirectory, not to
"/". This makes the home screen useful and things generally less confusing.
  - For unparsed commits, show a more descriptive error message without the
"blah blah" silliness.
  - For paths outside of the subpath parse tree, short circuit into an
appropriate error message.
  - For foreign SVN stub commits (see D892), show an explicit message.

Test Plan: Looked at unparsed commits, subpath repositories, foreign stub
commits, and paths outside of the subpath parse tree. Received sensible error
messages.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 894
2011-09-04 16:18:28 -07:00
Nicholas Harper
bd2b557b42 Mark diffusion browse result as empty directory when appropriate
Summary:
When selecting children of a directory, it is possible that none of its
children exist anymore even though the directory still exists. After fetching
the children but before returning them, we should check whether there are any,
and if there are no children, set the reason as empty directory.

Test Plan:
In sandbox, browsed in diffusion to a directory that exists but has no
files and saw that it has a useful message instead of a vague exception.

Reviewers: epriestley, tuomaspelkonen, jungejason

Reviewed By: tuomaspelkonen

CC: aran, tuomaspelkonen

Differential Revision: 846
2011-08-22 14:28:36 -07:00
epriestley
b8e08f34f7 Provide an indirection layer between documents and the search engine
Summary:
In preparation for adding another search engine (see T355):

  - Rename "executor" to "engine".
  - Move all engine-specific operations into the engine. Specifically, this
means that indexing moves out of the document store and into the engine (it was
sort of silly where it was before).
  - Split choice of an engine into an overridable "selector" class, a base API,
and a concrete MySQL implementation (just like storage engine selection).
  - Make all callers go through the indirection layer.

The default selector just unconditionally selects the MySQL engine, but now
(with D786) I can build an Elastic Search engine and you guys can build a
multi-target engine if you want and I don't get there fast enough.

Test Plan:
  - Created a new document (task).
  - Searched for and found it.
  - Viewed index reconstruction.

Reviewed By: jungejason
Reviewers: jungejason, amckinley, tuomaspelkonen, aran
CC: aran, jungejason, epriestley
Differential Revision: 788
2011-08-08 11:43:05 -07:00
epriestley
8a03a73e95 Fix some brace lint stuff.
Summary: New brace linter picked these up (see D755).
Test Plan: Visual inspection.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, jungejason
Differential Revision: 756
2011-08-02 10:40:45 -07:00
epriestley
35d03d36c7 Improve display behavior of commit messages in Diffusion
Summary:
See T372. Always render commit messages on one display line, so the table
doesn't jump around as they AJAX in on browse views.

The goal here is to have the cell choose a size naturally and for its content to
render with "overflow: hidden" if the natural size isn't large enough to contain
the content. "white-space: pre" or "white-space: nowrap" would prevent wrapping
but potentially make the table exceed the display width when a better behavior
is to hide some of the commit message.

Also use utf8-aware shortening, now that we have a function for it.

Casting a wide net in case anyone has a better way to do the CSS here. It's kind
of nasty that we have to use so many DOM nodes.

Test Plan:
  - Resized window while viewing browse and history views in Safari, Chrome and
Firefox. Table exhibited described behavior.
  - Verified summaries render sensibly and are properly truncated to 100
characters.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen, tomo, mroch, cpojer
CC: aran, epriestley
Differential Revision: 750
2011-07-31 12:05:06 -07:00
epriestley
ede78b2ccc Improve Diffusion behavior for SVN file moves
Summary:
We just weren't handling these at all reasonably, must have dropped the logic
when they got ported.

This still isn't perfect: we have some display glitches around file names, so
the 'away' part renders as "This file was moved to .". I'll see if I can follow
up and fix that, but this resolves the more immediate issue of the interface
just not working at all.

Test Plan: Moved and copied files in my test repository, verified they rendered
somewhat correctly.
Reviewed By: jungejason
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, jungejason, epriestley
Differential Revision: 699
2011-07-26 17:52:38 -07:00
Jason Ge
4dc6552af9 Restore "author" link to diffusion
Summary: create the page by getting data from the search result.
Test Plan:
load page with url /author/, /author/valid_username, and
/uathor/invalid_username, and verified that it works as expected.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
Commenters: tuomaspelkonen
CC: hwang, aran, tuomaspelkonen, epriestley, jungejason
Differential Revision: 723
2011-07-26 12:02:50 -07:00
epriestley
3eafe9e3bb Fix Diffusion rendering of SVN files which did not change
Summary:
Share code with the new PhabricatorDifferenceEngine, which handles diffs with no
changes correctly.

(This isn't the same issue as file moves, but I ran into it while generating a
repro case.)

Test Plan: Previously, changes which didn't change file content (e.g., property
changes) would throw. Now they work.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 698
2011-07-20 11:54:33 -07:00
epriestley
c0ae2f6289 Show change diffs in Phriction
Summary:
This is really rough and needs work (particularly, there's some diff code I
really need to refactor since I sort-of-copy-pasted it) but basically
functional.

Show text changes between diffs and allow users to revert to earlier versions.

Differential's line-oriented diff style isn't ideal for large blocks of text but
I'm betting this is probably good enough in most cases. We can see how bad it is
in practice and then fix it if needbe.

I added a bunch of support for "description" but didn't add the feature in this
diff, I'll either follow up or task it out since it should be a pretty
straightforward change.

Test Plan: Looked at history for several Phriction documents, clicked "previous
change" / "next change", clicked revert buttons.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen
CC: aran, hsb, epriestley
Differential Revision: 687
2011-07-18 08:46:45 -07:00
Blazej Osinski
3771f63441 Linking the unixname in Phabricator's blame view to the employee's profile.
Test Plan:
Visit page
http://phabricator.devXXXX.snc6.facebook.com/diffusion/E/browse/tfb/trunk/www/hphp_files.mk?view=blame
and verify that unixnames are hyperlinks leading to emplyees' profiles.

Task: T297

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, epriestley
Commenters: epriestley
CC: aran, blazej0, epriestley, tuomaspelkonen
Differential Revision: 622
2011-07-12 14:28:45 -07:00
epriestley
a49138defd Generalize the markup engine factory
Summary:
This thing services every app but it lives inside Differential right now. Pull
it out, and separate the factory interfaces per-application.

This will let us accommodate changes we need to make for Phriction to support
wiki linking.

Test Plan: Tested remarkup in differential, diffusion, maniphest, people,
slowvote.
Reviewed By: hsb
Reviewers: hsb, codeblock, jungejason, tuomaspelkonen, aran
CC: aran, hsb
Differential Revision: 646
2011-07-11 16:36:30 -07:00
epriestley
85b34c23f9 Clean up Phabricator interface to syntax highlighting
Summary: Reduce the amount of code duplication here and allow for an override
configuration on the filename.map stuff.
Test Plan: Checked paste, diffusion and differential syntax highlighting and
everything appeared reasonable.
Reviewed By: codeblock
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, codeblock, epriestley
Differential Revision: 601
2011-07-06 12:35:36 -07:00
epriestley
70cd8b1b34 Update Phabricator for split syntax highlighting API
Summary: I'll clean some of this stuff up in a followup too, but update the
callers to use the new explicit filename-based API.
Test Plan: Looked at paste, Diffusion and Differential.
Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, codeblock, jungejason, aran
CC: aran, tuomaspelkonen
Differential Revision: 600
2011-07-06 11:56:37 -07:00
Andrew Toulouse
9b522982fa Add timezone support
Summary:
Allows user-configurable timezones. Adds a preference panel, and migrates to the
new date rendering in easily-modified areas of the code. ***In progress***.

Test Plan:
Check database to make sure the field is being changed when the settings are
changed; check affected views to see how they render times.

Reviewed By: epriestley
Reviewers: epriestley
CC: aran, epriestley, toulouse
Differential Revision: 475
2011-06-18 13:07:43 -07:00
epriestley
58ed932e53 Fix some small Diffusion file browse view bugs
Summary:
"--date short" was introduced to git somewhere between 1.7.2.2 and 1.7.3.4,
despite 1.7.2.2 saying "--date <format>" in "git help blame". The older version
of git accepts "--date=short", however.

Also, the URI construction means you get "?view=" if you click a line number to
get a deep link, which I found vaguely annoying. Drop 'view' if we don't need
it.

Test Plan:
Looked at blame in my sandbox, although it worked before the date patch since I
have 1.7.3.4. Clicked a line number. Switched viewmodes.

Reviewed By: codeblock
Reviewers: codeblock, jungejason, tuomaspelkonen, aran
CC: aran, codeblock
Differential Revision: 423
2011-06-09 15:53:40 -07:00
Andrew Gallagher
b8194202e6 diffusion: fix git log author name parsing
Summary:
The current git log parsing wouldn't work correctly when space
appeared in the author name.  This diff fixes to regex to work
with multi-word author names and increases the author column
width when viewing blame info to accound for larger author names.

Test Plan:
viewed file with blame info in diffusion where author
name contained multiple words

Reviewed By: jungejason
Reviewers: jungejason, epriestley
CC: aran, rm, jungejason
Differential Revision: 399
2011-06-06 18:17:40 -07:00
Aizat Faiz
6c3b1feec8 [diffusion] keep 'view' parameter when clicking on a line number
Summary:
Clicking on a line number will remove the current 'view' the user is in.

This patch retains the current view.

Test Plan:
Open a file in diffusion, and change the view to "blame", clicking on the line
number should retain the same view.

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley, aizatto
Differential Revision: 344
2011-05-25 19:14:51 -07:00
epriestley
386a5eecb7 Show description changes in Maniphest
Summary:
When a task description is updated, there's currently no way to see the change.
Build an "expanded summary" mode for transactions that shows description change
details. Also include changes in the email.

Test Plan:
Changed task descriptions, clicked "show details", read email.

Reviewed By: aran
Reviewers: tuomaspelkonen, jungejason, aran
CC: anjali, aran, epriestley
Differential Revision: 320
2011-05-21 21:17:45 -07:00
tuomaspelkonen
59bfd17c61 Fixed history view.
Summary:
A lot of history views were empty. This fixes that problem.

Test Plan:
Played with sandbox.

Reviewed By: epriestley
Reviewers: epriestley, jungejason
Commenters: aran
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 316
2011-05-20 13:19:20 -07:00
tuomaspelkonen
22b2db6c15 Changed to use getBool and fixed pagesize and offset handling.
Summary:
Simplified code and now pressing 'Hide/Show' button doesn't lose the
pagesize/offset information.

Test Plan:
Tested with different arguments in my sandbox. Tested that the old
'copies=true' and 'copies=false' are still working.

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: aran, epriestley, tuomaspelkonen
Differential Revision: 318
2011-05-20 13:15:32 -07:00
epriestley
5fdea30878 Documentation: improve Diffusion documentation 2011-05-19 13:40:40 -07:00
epriestley
6b8da8c347 Fix Diffusion line number links in Git
Summary:
When you click a line number link in Git from a branch tip, it takes you to
"...;origin/master$..." which (a) doesn't work and (b) doesn't permanently
reference the line.

Link to the "stable commit name" instead.

Also fix a few other bugs/warnings/layout things.

Test Plan:
Clicked line number links in Git and SVN repositories, browsed around stuff,
checked error log.

Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 303
2011-05-18 09:24:50 -07:00
epriestley
54154e4f48 Move "Rendering References" to the DifferentialChangesetParser level
Summary:
Separates changeset IDs from rendering. Now each changeset has a "rendering
reference" which is basically a description of what the ajax endpoint should
render. For Differential, it's in the form "id/vs". For Diffusion,
"branch/path;commit".

I believe this fixes pretty much all of the bugs related to "show more" breaking
in various obscure ways, although I never got a great repro for T153.

Test Plan:
Clicked "show more" in diffusion change and commit views and differential diff,
diff-of-diff, standalone-diff, standalone-diff-of-diff views. Verified refs and
'whitespace' were always sent correctly.

Made inline comments on diffs and diffs-of-diffs. Used "Reply".

Reviewed By: tuomaspelkonen
Reviewers: tuomaspelkonen, jungejason, aran
CC: aran, tuomaspelkonen, epriestley
Differential Revision: 274
2011-05-13 06:10:57 -07:00
jungejason
e8744e7eeb Enable pygment highlighting for diffusion
Summary:
set the config for the diffusion highlighter. In D202 we
enabled it for differential already.

Test Plan:
opened a python file in diffusion and verified it is highlighted.

Reviewed By: epriestley
Reviewers: epriestley, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 260
2011-05-10 16:13:20 -07:00
tuomaspelkonen
d63f3d479c Fixed 'Modified' link in Diffusion History View.
Summary:
'Modified' link always pointed to the latest change. Fixed the link.

Test Plan:
Tested that clicking 'Modified' took me to the correct place. Made sure that
Diffusion commit view was still working correctly.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, aran, epriestley
Differential Revision: 255
2011-05-09 16:53:59 -07:00
Aizat Faiz
a8296c5a54 Display file name in <title> for easier browsing of code
Summary:
Having the filename in the <title> will make it easier to browse through many
many tabs of diffusion

Test Plan:
Open up a file or directory in diffusion and ensure that it shows only the file
name.

Reviewed By: tuomaspelkonen
Reviewers: epriestley, tuomaspelkonen, jungejason
Commenters: epriestley
CC: epriestley, aizatto, tuomaspelkonen
Differential Revision: 150
2011-04-21 17:22:47 -07:00
tuomaspelkonen
a6b873692f 100-file cutoff for Diffusion
Summary:
If there are more than 100 changed files in a single commit in
Diffusion, only the first 100 changes will be shows. There is a warning
sign about this and a button that will reload the same page with all the
changes visible.

Test Plan:
Tested that everything worked as expected with commits over and under 100
commits.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley
Differential Revision: 110
2011-04-07 16:56:25 -07:00
tuomaspelkonen
a34cb4bb12 Added button for showing/hiding copies in Diffusion history view
Summary:
By default, indirect events in Diffusion SVN history views were visible.
Now they are invisible, but the visiblity can be changed using a button
in the top right corner.

Test Plan:
Tested on multiple different files that the history is shown correctly after
the page is loaded, after the button is clicked once and after the button is
clicked the second time.

Reviewed By: epriestley
Reviewers: epriestley
CC: jungejason, epriestley, tuomaspelkonen
Differential Revision: 106
2011-04-06 19:08:11 -07:00
epriestley
e92b1fee9f Show child changes in last-modified view. 2011-04-06 19:06:00 -07:00
epriestley
1910f43364 Fix LastModified query under SVN to ignore indirect changes. 2011-04-06 12:28:16 -07:00
epriestley
71b2f4a567 Fix a minor path encoding issue with git branches. 2011-04-03 23:42:33 -07:00
epriestley
736f629a07 Lint stragglers. 2011-04-03 23:38:02 -07:00
epriestley
23f882a0ee Some owners write workflows. 2011-04-03 22:03:27 -07:00
epriestley
5038ab850c Some owners read workflows. 2011-04-03 19:20:47 -07:00
epriestley
65e1386753 owners 2011-04-03 15:50:51 -07:00
epriestley
1b2140d7a4 Unmuck some issues with viewing HEAD changes. 2011-04-02 17:12:41 -07:00
epriestley
8f5d01d451 Get rid of +x on a bunch of nonexecutable files because I failed to set
"create mask" on SMB. :/
2011-04-02 16:47:20 -07:00
epriestley
811a0910e0 Pull handles for the standalone browse view. 2011-04-02 16:44:19 -07:00
epriestley
e4cd939734 Do author PHID lookups in Diffusion views. 2011-04-02 16:39:41 -07:00
jungejason
313da1d5eb Support blame on blame for svn
Summary:
add the column for the blame on blame for svn. We will support
git once we have the 'parent' info of the commits saved in the database
for git.

Test Plan:
in svn it should work. In git is doesn't break things.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 95
2011-04-01 20:11:08 -07:00
epriestley
82fffe466a Add basic detail-parser functionality. 2011-04-01 17:11:55 -07:00
jungejason
d23696a457 Add image support for diffusion file view
Summary:
render image as an image tag in diffusion view.

Test Plan:
1. Image shows up correctly. 2. Non-image file still works
fine.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 94
2011-04-01 16:20:36 -07:00
jungejason
64cd4f969d Add color to diffusion blame and improve plain view
Summary:
query the database to get the epoch info for the commits, then
calculate the color depending on the epoch (the newer the commit, the
dark its color). Also improved the plain blame view for git, as the
git-blame doesn't produce a good display by default. Now we format the
output it from the data we fetches from the database.

Test Plan:
verify both git and svn browsing page work for 'plain',
'plainblame', 'highlighted' and 'highlightedblame' view.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley, jungejason
Differential Revision: 93
2011-04-01 10:25:36 -07:00
epriestley
3369147cab Paginate Diffusion history views, respect 'pagesize' parameter. 2011-03-31 18:46:53 -07:00
tuomaspelkonen
28fe9f4eca User preferences ported from tools
Summary:

Internal tools, e.g., differential and diffusion  have user defined
preferences for monospaced font and the option for showing either the
name of the tool or the glyph of the tool in the title.

These preferences were ported to phabricator. These preferences can be
modified in /preferences/ and they both affect diffusion and differential
at the moment.

Test Plan:

* Created an empty database
* Loaded /preferences/ and modified the monospaced font and clicked save
* Confirmed that the same page was loaded with the message that preferences
  have been saved and that the example text used the user defined font

* in /preferences/ changed the option to show tool names as plain text and
  clicked save
* Confirmed that the same page was loaded with '[Preferences]' in the title
  instead of a glyph

* These same tests were also executed for differential and diffusion

Reviewers: epriestley
CC: jungejason

Differential Revision: 91
2011-03-31 13:44:20 -07:00
epriestley
c795aaea29 Make the branch table somewhat more useful. 2011-03-31 01:07:51 -07:00
epriestley
34e8d902c7 Restore "Shortcuts" feature to Diffusion. 2011-03-31 00:33:44 -07:00
epriestley
c99df1f4eb Add more change metadata to SVN and git. 2011-03-30 23:27:06 -07:00
epriestley
e25c58ed9c Restore some commit metadata to browse views. 2011-03-30 22:41:31 -07:00
epriestley
e6cf7a9cb0 More Diffusion junk. 2011-03-30 22:08:41 -07:00
epriestley
b266b570d8 Use remarkup engine for commit info. 2011-03-30 21:37:28 -07:00
epriestley
e994e69a6f Limit the amount of damage we cause to ArcanistDiffParser. 2011-03-30 19:32:51 -07:00
epriestley
60918c9a9e Improved change view support. 2011-03-30 19:22:11 -07:00
epriestley
5063e978f1 Basic DiffusionGitDiffQuery. 2011-03-30 18:17:36 -07:00
epriestley
3d5f03607b Rough cut of Diffusion change view. 2011-03-30 17:36:32 -07:00
jungejason
51a6ce65aa Show blame info in diffusion.
Summary:
Show blame info. This is part of the task of "Port Diffusion's
Browse File view to Phabricator". The color for git repository is not
implemented yet.

Test Plan:
it would work for both git and svn.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 87
2011-03-30 16:07:57 -07:00
epriestley
fa8f168bd0 Allow commits to be marked as 'bad' so they won't be parsed. Useful if you work
at a company where someone deleted the entire repository *cough cough*.
2011-03-26 23:52:09 -07:00
jungejason
1844a6f728 Get the correct uri_path for a diffusion request.
Summary: there is a bug in getting the uri path. When the user clicks
a line number twice, the new rev number and the line number is attached
to the end of the original uri instead of substituting it.

Test Plan: clicking line number multiple times, for both git and svn.

Reviewers: epriestley

CC:

Differential Revision: 84
2011-03-26 00:17:14 -07:00
jungejason
431552c357 Add syntax highlight to diffusion.
Summary:
use XHPAST parser to parse the file, and generate a table for
the code to highlight it.  This is part of the task of "Port Diffusion's
Browse File view to Phabricator".

Test Plan:
browse file, try commit version, line number functionality.

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 83
2011-03-25 17:41:51 -07:00
jungejason
daf7a07064 Add dropdown and make plain text view style.
Summary:
part of the task of "Port Diffusion's Browse File view to
Phabricator"

Test Plan:
view the plain view style

Reviewed By: epriestley
Reviewers: epriestley
CC: epriestley
Differential Revision: 80
2011-03-22 13:12:55 -07:00
epriestley
afe0079819 Improve awkward Diffusion query plans. 2011-03-20 17:46:02 -07:00
epriestley
01a20c0480 Fix various parsing bugs in Differential. 2011-03-19 14:42:17 -07:00
epriestley
bb144542d3 Diffusion/phd/console improvements. 2011-03-15 13:38:14 -07:00
epriestley
5970f9a0ec Get rid of git.path configuration, this is really an artifact of my system
being broken and probably macports' fault.
2011-03-14 09:43:06 -07:00
epriestley
4196bfb3ef Minor fixes. 2011-03-14 07:43:13 -07:00
epriestley
a3cd33da19 Lint stragglers. 2011-03-13 22:12:34 -07:00
epriestley
b5db12b6b2 Browse / git improvements. 2011-03-13 22:03:30 -07:00
epriestley
7963d7fa06 Lint & packaging. 2011-03-13 20:15:24 -07:00
epriestley
426754dd4d Improve Diffusion commit view featureset. 2011-03-13 20:12:46 -07:00
epriestley
c7d343e05e Some Diffusion path change stuff. 2011-03-13 16:20:05 -07:00
epriestley
4893146815 Improve parser scalability, fix a bug or two, provide 'phd', the Phabricator
Daemon interface.
2011-03-13 14:27:03 -07:00
epriestley
383b3d740c Improve some diffusioney things.
Summary:

Test Plan:

Reviewers:

CC:
2011-03-12 22:51:40 -08:00
epriestley
485b5e5ded Make the Diffusion UI vaguely usable in some cases. 2011-03-12 16:17:34 -08:00
epriestley
48b5d6d025 Diffusion change parser for SVN.
Summary: there is a small possibility this works correctly

Test Plan:

Reviewers:

CC:
2011-03-11 14:13:23 -08:00
epriestley
8bcbeface1 Increase parseriness of some parsery things.
Summary: None of these are parsnips.

Test Plan:

Reviewers:

CC:
2011-03-11 09:34:22 -08:00
epriestley
c82cab35e2 Diffusion: rough cut of history view
Summary:
Very very rough approximation of history view. I left out all the
log parsing stuff for now since we should be able to just look it up in
a Repository table and I think that'll be a bit faster, although we can
muck around and see.

Test Plan:
Looked at history of a path

Reviewed By: jwilson
Reviewers: aran, jwilson
CC: epriestley, jwilson
Differential Revision: 66
2011-03-10 13:47:28 -08:00
epriestley
958b00c010 Diffusion: encapsulate request in DiffusionRequest
Summary:
Put an indirection layer between controllers and URI management,
adding branches to git repositories.

Test Plan:
Looked at browse, history browse, file browse views, bad branches,
bad commits

Reviewed By: jwilson
Reviewers: aran, jwilson
CC: jwilson, epriestley
Differential Revision: 65
2011-03-08 16:11:42 -08:00
epriestley
c9a4820abf Diffusion: basic file browse capability
Summary:
Very rough cut of file browsing. Not terribly useful yet, but it does
cause file data to appear in the browser window.

Test Plan:
viewed a file from a git repo

Reviewed By: jwilson
Reviewers: aran, jwilson
CC: jwilson
Differential Revision: 64
2011-03-08 10:44:04 -08:00
epriestley
20bcb9379e Phabricator Diffusion: handle missing path errors in browse view
Summary:
When the user gets a path wrong, show them a helpful error message, as
in Diffusion/Confusion.

Test Plan:
Browsed some nonexistent and previously-deleted paths.

Reviewed By: aran
Reviewers: aran, jwilson
CC: aran
Differential Revision: 59
2011-03-07 22:03:23 -08:00
epriestley
0d8bac97ae Diffusion: basic browse view
Summary:
Synthesizes elements of Diffusion's browse view, Confusion's git
support and Phabricator's repository infrastructure to provide a basic browse
view for Phabricator Diffusion.

This is basically a straight port of Confusion but uses Phabricator's
Repository object and uses a real data object instead of arrays.

Test Plan:
Browsed Javelin in Phabricator at a very basic level.

Reviewed By: jwilson
Reviewers: aran, jwilson
CC: jwilson, epriestley
Differential Revision: 58
2011-03-07 18:28:39 -08:00
epriestley
7c413fc580 Rough version of DiffusionHomeController
Summary: My strategy here is to synthesize Diffusion + Confusion (jwilson's
git support for Diffusion) + Repository/Commit stuff into Phabricator and
kill all birds with one stone. This probably involves the least total work
since we have to do this port step anyway and a lot of the Phabricator
primitives are in better shape than the trunk primitives.

Test Plan: Looked at it in my sandbox, which has some parsed commits.

Reviewers: jwilson, aran, jungejason

CC:

Differential Revision: 57
2011-03-07 16:21:11 -08:00