Summary:
Ref PHI1749. Instead of opening files to the last unchanged line on either side of the change, open files to the "simple" line number of the selected block.
For inlines, this is the inline line number.
For blocks, this is the first new-file line number, or the first old-file line number if no new-file line number exists in the block.
This may not always be what the user is hoping for (we can't know what the state of their working copy is) but should produce more obvious behavior.
Test Plan:
- In Diffusion, used "Open in Editor" with and without line selections. Saw same behavior as before.
- Used "n" and "r" to leave an inline with the keyboard, saw same behavior as before.
- Used "\" and "Open in Editor" menu item to open a file with:
- Nothing selected or changeset selected (line: 1).
- An inline selected (line: inline line).
- A block selected (line: first line in block, per above).
Differential Revision: https://secure.phabricator.com/D21282
Summary: Depends on D20345. Use a narrower layout for very large graphs to save some space.
Test Plan:
Before:
{F6320215}
After:
{F6320216}
This does not affect smaller graphs.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D20346
Summary: Ref T13105. After moving Diffusion to DocumnentEngine, this no longer has callers. It will become part of the document behavior.
Test Plan: Grepped for calls to the `diffusion-browse-file` behavior, found none.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13105
Differential Revision: https://secure.phabricator.com/D19377
Summary:
Depends on D19349. Ref T13105. This was the behavior in Diffusion before with a little hard-coded snippet.
Remove that snippet ("diffusion-jump-to") and add a more general-purpose snippet to SourceView.
This is a tiny bit hacky still (and probably doesn't work quite right with Quicksand) but gets things working again and works in all of Files, Paste, and Diffusion.
Test Plan: Followed links to particular lines in Paste, Files and Diffusion; got scrolled to the right place.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13105
Differential Revision: https://secure.phabricator.com/D19350
Summary: Ref T13105. This needs refinement but blame sort of works again, now.
Test Plan: Viewed files in Diffusion and Files; saw blame in Diffusion when viewing in source mode.
Reviewers: mydeveloperday
Reviewed By: mydeveloperday
Maniphest Tasks: T13105
Differential Revision: https://secure.phabricator.com/D19309
Summary: Noticed a couple of typos in the docs, and then things got out of hand.
Test Plan:
- Stared at the words until my eyes watered and the letters began to swim on the screen.
- Consulted a dictionary.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D18693
Summary: Updates to use the standard 13px size we use everywhere else, cleans up a few mobile/display bugs, adds a hover state for `tr`.
Test Plan:
Review Diffusion, Daemons, Almanac, People Logs, anything else?
{F4991070}
{F4991071}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D18088
Summary:
Fixes T12599. If you're faster than the network request, we don't actually resolve your query when the data arrives.
Instead, when the data shows up, run the query if the user has typed something.
Test Plan: Pasted a filename in real fast, got results. (Previously: no results until you press something else.)
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12599
Differential Revision: https://secure.phabricator.com/D17739
Summary:
Ref T4788. This fixes all the bugs I was immediately able to catch:
- "Directory-Like" graph shapes could draw too many vertical lines.
- "Reverse-Directory-Like" graph shapes could draw too few vertical lines.
- Terminated, branched graph shapes drew the very last line to the wrong place.
This covers the behavior with tests, so we should be able to fix more stuff later without breaking anything.
Test Plan:
- Added failing tests and made them pass.
{F1708158}
{F1708159}
{F1708160}
{F1708161}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4788
Differential Revision: https://secure.phabricator.com/D16216
Summary: Fixes T10816. The way these work is a little unusual since these chunks of file-rendering code are unusuall performance-sensitive, so the Differential version doesn't adapt directly to Diffusion. Both can possibly be unified at some point in the future, although they do slightly different things.
Test Plan: {F1220170}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10816
Differential Revision: https://secure.phabricator.com/D15719
Summary:
Fixes T9323. Two minor fixes:
- On the first commit, don't render a downward line.
- Clean up a 1px spacing issue that had cropped up a while ago when we added icons or something, I think.
Test Plan:
Before:
{F1057248}
After:
{F1057249}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9323
Differential Revision: https://secure.phabricator.com/D14974
Summary:
Ref T9524. Because fetching the last time files were modified in Diffusion can be slow, we bring it in over Ajax.
The logic to fetch and paint the table is kind of fragile because there are two different definitions of the columns right now and we break in a bad way if they differ.
In particular, calling `diffusion.updatecoverage` can populate a "lint commit" for a repository, which tries to generate lint information in one of the views (but not the other one).
In the longer run I think we're removing some of the concepts here and this rendering should be rebuilt to not have two separate column definitions, but just make it degrade gracefully for now since those are larger changes.
Test Plan:
Reproduced the issue in T9524 by calling `diffusion.updatecoverage` on a repostiory. Specifically, this has a side effect of creating a "lint commit" which triggers a "lint" column in this table, sort of.
Applied this patch, got a clean render.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9524
Differential Revision: https://secure.phabricator.com/D14243
Summary: I considered at the time just making all tables taller. This removes the special casing and adds the space universally. On first glance all smaller tables look great, but Diffusion seems a little bloated. After a short time period though that went away for me. I do think Diffusion overall needs a UI refresh.
Test Plan: Tested numerous tables in Phortune, Diffusion, etc. Spacing feels more readable.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12328
Summary: Fixes T7084. This doesn't use the same anchor logic as other applications.
Test Plan: `$245` lines now jump to line 245 on page load.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7084
Differential Revision: https://secure.phabricator.com/D11563
Summary: Fixes T7033. When we've reframed the main page content we need to scroll relative to the containing frame, not relative to the window.
Test Plan:
In Safari, Chrome and Firefox, used j/k/J/K keys to navigate diff content.
Tried some other scroll-based beahviors, like jump-to-anchors.
(It looks like the highlighting reticle got slightly derped a while ago, but it's still functional, so I didn't mess with it.)
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7033
Differential Revision: https://secure.phabricator.com/D11490
Summary: As advised by JSHint.
Test Plan: I'm not really sure how to comprehensively test this. It looks okay to me.
Reviewers: chad, epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9677
Summary: There are a bunch of unused variables in JavaScript files. These were identified with JSHint.
Test Plan: It's pretty hard to test this thoroughly... on inspection, it seems that everything //should// be okay (unless we are doing weird things with the JavaScript).
Reviewers: chad, #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9676
Summary:
Allows you to quickly search for files within a repository. Roughly:
- We build a big tree of everything and ship it to the client.
- The client implements a bunch of Sublime-ish magic to find paths.
Test Plan: {F154007}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D9087
Summary:
This code is currently quite complicated because we pull history data inline for SVN files, and via ajax for everything else (SVN dirs, everything in Git and Hg).
Always pull over ajax; batch some of the queries.
Test Plan: {F34860}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, vrana, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5255
Summary: Ref T2683. Instead of sending one request for each path's history, send one request for all of it. This permits optimizations which are not currently available to us. It degrades the user experience a tiny bit in theory, but on my machine it's actually way faster already.
Test Plan: Loaded a browse page.
Reviewers: vrana, btrahan
Reviewed By: btrahan
Subscribers: epriestley, aran
Maniphest Tasks: T2683
Differential Revision: https://secure.phabricator.com/D5254
Summary:
Add the ability to select singular and multiple lines in paste to highlight.
This is related to T3627
Test Plan: Create a paste, select one or more lines.
Reviewers: epriestley, tberman
Reviewed By: epriestley
CC: aran, chad
Maniphest Tasks: T3627
Differential Revision: https://secure.phabricator.com/D6668
Summary: If `jsxmin` is not available, use a pure PHP implementation instead (JsShrink).
Test Plan:
- Ran `arc lint --lintall` on all JS and fixed every relevant warning.
- Forced minification on and browsed around the site using JS behaviors. Didn't hit anything problematic.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5670
Summary:
I have blame enabled by default and displaying files with long history takes easily over 10 seconds.
Load the blame data by AJAX instead.
This is actually doing more work and the total response time is longer but it's worth it for me as I am interested just in the file contents quite often.
I know you were talking about building blame cache but until we have it...
I'm not sure if the AJAX loading indicator in bottom right corner is enough to inform the user that we are loading it on background.
Test Plan:
?view=highlighted
?view=plainblame
?view=blame
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5244
Summary:
For a single line, I can use the right click.
But for highlighting multiple lines, I have to wait for page reload.
It would be nice to track this in history but that's more involved.
This is actually maybe better behavior.
Test Plan: Clicked on the line link, dragged and dropped on it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5172
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
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
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
Summary: -
Test Plan:
This was a pretty straightforward replace. Everything should
be sane.
Reviewed By: epriestley
Reviewers: tomo, epriestley, mroch
CC: aran, epriestley
Differential Revision: 803
Summary:
Javelin is currently embedded in Phabricator via copy-and-paste of prebuilt
packages. This is not so great.
Pull it in as a submodule instead and make all the Phabriator resources declare
proper dependency trees. Add Javelin linting.
Test Plan:
I tried to run through pretty much all the JS functionality on the site. This is
still a high-risk change, but I did a pretty thorough test
Differential: inline comments, revealing diffs, list tokenizers, comment
preview, editing/deleting comments, add review action.
Maniphest: list tokenizer, comment actions
Herald: rule editing, tokenizers, add/remove rows
Reviewed By: tomo
Reviewers: aran, tomo, mroch, jungejason, tuomaspelkonen
CC: aran, tomo, epriestley
Differential Revision: 223
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
