Summary: For "accept" and "reject" action, find the time between the action and last time the revision was updated by a new diff. Show it as the responsiveness row.
Test Plan: view it for a couple of engineers.
Reviewers: epriestley, vii
Reviewed By: epriestley
CC: vrana, nh, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2970
Summary:
- Assigning $cc_phids clobbers the correct value assigned on line 80.
- Remove stack trace noise, the trace is always meaningless and well-known.
- Actually show the original body.
Test Plan: Piped mail to the mail receiver and verified the errors didn't CC revision CCs, no longer had traces, and included the original raw text body.
Reviewers: btrahan, jungejason
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2969
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
Summary: Did exactly what @epriestley suggested in T1428#2.
Test Plan: Turn it on in your config, post a revision, accept it. Turn it off in your config, post a revision, can't accept it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2900
Summary:
Diff of diffs display changes between new versions of file.
This is bad after rebase because there can be many unrelated changes so it is hard to spot the real change.
This diff unhighlights the lines that were added or removed in rebase.
The changes are still visible (they can be sometimes relevant) but very subtle.
Test Plan:
# Add, change and delete line. Display diff.
# Add and change some lines in parent. Rebase. Display diff. Display diff of diff.
# Change and add some lines. Display diff. Display diff to first diff. Display diff to second diff.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: jungejason, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2761
Summary:
This adds:
1) A new "arc:lint-postponed" diff property which stores a list of
lint names that are postponed and a finishpostponedlinters conduit
method which removes linters from this list. Postponed linters are
shown in the lint details.
2) A updatelintmessages conduit message, which adds additional lint
messages to the "arc:lint" diff property.
In combination, this provides very basic support for running
asynchronous static analysis tools. When the diff is being created,
a list of asynchronous static analysis runs can be added to the
diff's postponed linters list. As these postponed linters finish
up, then can report new lint messages back to the diff then mark
themselves as complete.
The client is currently responsible for filtering the lint messages
by things like affected lines and files.
Test Plan:
Used conduit call API to add lint messages and remove postponed
linters from a test diff.
Reviewers: epriestley, vrana, nh, jungejason
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1332
Differential Revision: https://secure.phabricator.com/D2792
Summary: people want to get the info about how long the revision has been active since the last time the authoer ran 'arc diff'.
Test Plan: call it from conduit console
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: Girish, aran, epriestley
Differential Revision: https://secure.phabricator.com/D2885
Summary: this lets people download diffs easily in case arc export, arc patch, etc isn't to their liking or whatever.
Test Plan: "Download Raw Diff" a few times with various things toggled in the "Revision Update History" widget
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1294
Differential Revision: https://secure.phabricator.com/D2855
Summary: This error usually doesn't occur because we have only one hunk per changeset most of the time.
Test Plan:
$hunk = new ArcanistDiffHunk();
$hunk->setAddLines(1);
$hunk->setDelLines(1);
$change = new ArcanistDiffChange();
$change->addHunk($hunk);
$change->addHunk($hunk);
$diff = DifferentialDiff::newFromRawChanges(array($change));
var_dump($diff->getLineCount());
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2878
Summary: I will also need `getRemovedLines()` so refactor this first.
Test Plan:
New test case.
Viewed uncached diff.
Verified that the only callsite of `getAddedLines()` trims lines.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2875
Summary: When the commandeerer was a reviewer, after the commandeering, he stayed as a reviewer. He can no longer amend the diff. He has to go 'Edit Revision' to remove himself/herself. The fix is to remove it automatically.
Test Plan:
comamndeereded a revision and the behavior is correct now:
- I was removed from the revision list
- the comment transaction shows one more entry that I removed myself as a reviewer
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: nh, vrana, aran, Korvin
Maniphest Tasks: T1225
Differential Revision: https://secure.phabricator.com/D2872
Summary: See T693. To do "arc branch" performantly in immutable history repositories, we need to be able to do a single query to identify revisions related by hash. Return hash information to enable this.
Test Plan: Ran `differential.query`.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T693
Differential Revision: https://secure.phabricator.com/D2859
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
Summary: People want to create filters checking if they are in CC which is almost impossible with multiplexing in Outlook.
Test Plan: Sent e-mail with multiple CCs, verified headers.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2829
Summary:
"differential.creatediff" requires a mostly-parsed diff, but there's no reason we can't make DifferentialDiffs out of raw diffs.
This mainly serves to lower the adoption barrier if getting "arc" distributed is too much of a hassle.
Test Plan: Made a diff out of a raw block of diff text.
Reviewers: ffx, vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2751
Summary: This prevents "8 active days" for last week.
Test Plan: `strtotime('1 week ago 24:00')`
Reviewers: john, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2794
Summary: `array_fill()`, contrary to `range()`, doesn't accept the last element but the number of elements.
Test Plan: Reparsed commit not changed after the last diff but rebased which was previously reported as changed.
Reviewers: jungejason, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2783
Summary:
Works this way:
- Select users' language with multiplexing.
- Select default language otherwise (it can be different from current user's language).
- Build body and subject for each user individually.
- Set the original language after sending the mails.
Test Plan:
- Comment on a diff of user with custom translation.
- Set default to a custom translation. Comment on a diff of user with default translation.
- Set default to a default translation. Comment on a diff of user with default translation.
Repeat with/without multiplexing.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1139
Differential Revision: https://secure.phabricator.com/D2774
Test Plan: Created action link using VS diff, selected VS diff.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2770
Summary: We need it.
Test Plan: Ran 'differential.getdiff', saw it.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2766
Summary:
This is to allow conservative people to bring back the old manners.
NOTE: It doesn't mark all occurrences but I think that's good enough. We will eventually mark them all.
Test Plan: Translated 'closed', displayed closed revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, asukhachev
Maniphest Tasks: T1139
Differential Revision: https://secure.phabricator.com/D2760
Summary:
This is the first step in Phabricator internationalization.
It adds a translation selector and calls it at startup.
Installations can add custom selectors to override some texts.
We can add official translations in future.
Next step is to allow user to choose his translation which will override the global one.
This is currently used only for English plurals.
Test Plan: Displayed a diff with unit test error, verified that it says 'Detail' or 'Details' and not 'Detail(s)'.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1139
Differential Revision: https://secure.phabricator.com/D2753
Summary:
Some e-mail clients display this header and it needs to be constant.
This is somehow involved but I doubt that there is a simpler solution.
Test Plan:
Applied SQL patch.
Commented on revision, commented on commit, changed package.
Verified that the `Thread-Topic` has constant and human readable value.
Reviewers: epriestley
Reviewed By: epriestley
CC: ola, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2745
Summary:
Added a dropdown menu button and the keyboard shortcut 'h' to the
web diff view. These hide or show the annotated code display.
Test Plan:
Viewed an example diff that changed a large number of source files
and played around with keyboard shortcuts. Everything seemed to
work as expected.
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D2714
Summary: NOTE: This can break current ongoing conversations.
Test Plan: Commented on a revision and checked the header in the e-mail.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1340
Differential Revision: https://secure.phabricator.com/D2723
Summary:
It seems that Outlook and Mail.app mostly ignores the threading headers and thread primarily by subject.
They are also very picky about the Re: part in the header.
I guess that's because users of these clients often hit Reply when they want to create a new message to the sender of an e-mail.
We need both of these applications to work with the same setting because we don't use multiplexing to prevent sending multiple e-mails to people in lists.
I also believe that the default behavior should just work in most setups.
I've tried several different combinations of putting "Re:" and none of them seems to always work in both clients.
This diff at least adds more abstraction to the code which should prevent copy/paste errors (two fixed by this diff!).
Test Plan: Sent several e-mails with varying subject, verified that they look as before in Outlook and Mail.app.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2709
Summary: The notification implementation has been extended to Differential. Appropriate changes have been made to the Differential editors and Differential feed story.
Test Plan: Tested out various actions available for Differential and confirmed that the notifications get delivered correctly and feed is generated.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2696
Summary:
I need this information quite often.
I don't know how many people are working on a single project and this information will be useless for them but I guess it won't hurt much?
Test Plan: Commented on accepted revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2674
Summary:
This also changes some stuff:
- Reviewers used to be at top, now they are under comments.
- Primary reviewer is now rendered as first.
Test Plan: Added `renderValueForMail()` to a custom field, created diff, commented on it and verified e-mails.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2664
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
Summary: D2216 tried to ask the user, this one is explicit.
Test Plan: Click the button
Reviewers: epriestley, lucian
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2600
Summary:
Some lint errors (e.g. Javelin) don't have a line number.
Put them on the first line.
Putting them above the first line would be even nicer but much more complicated.
Test Plan: Display diff with lint error on line 0 (D2583).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2599
Summary: Also fix the notice text.
Test Plan:
Display diff with inline comments and lint errors.
Click on inline comment and lint link.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2594
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
Summary:
I thought about it a little bit and this makes the most sense for me:
# Original author usually writes at least something and commander only updates it.
# There's a creation date of revision (= first diff) by these comments. I don't want to change this date because I use this information. Author should correspond to this date.
# It solves all our repros.
Test Plan: Display commandeered revision.
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2577
Summary:
Instead of assuming the test plan and summary are written by the author
of the differential revision, let's assume they are written by the author
of the latest differential diff.
Test Plan: viewed a drev that had been commandeered but not updated to check authors
Reviewers: epriestley, jungejason, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1235
Differential Revision: https://secure.phabricator.com/D2550
Summary:
If someone typos one name in a cc or reviewer list, it would be nice if we
display all of the valid names in the field when running arc diff (in addition
to the error message).
Test Plan:
used the conduit console to check that calling differential.parsecommitmessage
with a list of some valid and some invalid ccs returns a result with both an
error and a list of some ccs. Also ran arc diff with that list of ccs to check
for the correct user experience.
Reviewers: epriestley, jungejason, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2542
Summary:
This attaches commit diff to its associated revision as any other diff.
The consequence is that the revision page now shows the actual commit instead of the last diff. It may be disturbing but it is desired.
Another consequence is that lint and unit results are displayed as skipped after the revision was committed. I want to fix it somehow.
My next plan is to automatically diff against the last normal diff and include the link to this diff in commit e-mail if non-empty.
Test Plan:
reparse.php
Diff against last normal diff.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: aran, Koolvin, jungejason
Maniphest Tasks: T201
Differential Revision: https://secure.phabricator.com/D2530
Summary: Add support for "tests", "testplan" and "tested" as alias of "Test Plan".
Test Plan: Created a diff with test plan specified in "tests".
Reviewers: csilvers, btrahan
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2531
Test Plan: Display revision list both with last reviewer and without.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2495
Summary:
This is not so general as `getRequiredHandlePHIDs()`.
It allows bulk loading of user statuses only in revision list.
It also loads data in `render()`. I'm not sure if it's OK.
Maybe we can use the colorful point here.
Or maybe some unicode symbol?
Test Plan: {F11451, size=full}
Reviewers: btrahan, epriestley
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2484
Summary:
I've tried colors, italics, strike-through and this is a compromise between me and @leebyron.
NOTE: I don't want to disturb @epriestley from playing Diablo 3.
Test Plan: {F11439, size=full}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, leebyron
Differential Revision: https://secure.phabricator.com/D2481
Summary: the existing, more custom text doesn't make sense when viewing someone else's revisions AND in someplaces its the less interesting "no data found".
Test Plan: clicked around the various http://phabricator.dev/differential/filter/* pages with various users.
Reviewers: epriestley, ry
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1222
Differential Revision: https://secure.phabricator.com/D2475
Summary: We had a real issue where revision was marked as accepted but the comment wasn't saved.
Test Plan: Reclaim abandoned revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: jungejason, aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2463
Summary: Revisions don't necessarily have a backing repository, as in "--raw".
Test Plan: Looked at a "--raw" revision in accepted and closed states.
Reviewers: nh, btrahan
Reviewed By: nh
CC: aran
Differential Revision: https://secure.phabricator.com/D2444
Summary:
The highlighting is distracting according to Nick Shrock and others.
Real designer, Lee Byron, helped me with this.
It also gives us unagressive target for jumping to the source line in future.
Another feature I will probably implement is highlighting also the source of copies/moves.
I will use the right side of the left column for it.
Test Plan:
Hover copied notifier.
Hover coverage notifier.
I've also checked that this doesn't break our super-flaky old/new code JavaScript detector.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin, leebyron, schrockn
Differential Revision: https://secure.phabricator.com/D2443
Summary:
- Move email to a separate table.
- Migrate existing email to new storage.
- Allow users to add and remove email addresses.
- Allow users to verify email addresses.
- Allow users to change their primary email address.
- Convert all the registration/reset/login code to understand these changes.
- There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
- This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.
Not included here (next steps):
- Allow configuration to restrict email to certain domains.
- Allow configuration to require validated email.
Test Plan:
This is a fairly extensive, difficult-to-test change.
- From "Email Addresses" interface:
- Added new email (verified email verifications sent).
- Changed primary email (verified old/new notificactions sent).
- Resent verification emails (verified they sent).
- Removed email.
- Tried to add already-owned email.
- Created new users with "accountadmin". Edited existing users with "accountadmin".
- Created new users with "add_user.php".
- Created new users with web interface.
- Clicked welcome email link, verified it verified email.
- Reset password.
- Linked/unlinked oauth accounts.
- Logged in with oauth account.
- Logged in with email.
- Registered with Oauth account.
- Tried to register with OAuth account with duplicate email.
- Verified errors for email verification with bad tokens, etc.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2393
Summary:
- In practice, 'edit' has two modes, 'create' and 'edit'. These seem like they should map to "create a revision" and "update a revision", but they are completely different.
- We use the "create" mode:
- When creating a message from the working copy.
- When creating a message from a file.
- When creating a message from a commit.
- When creating a message from a user template.
- When creating a message from an "--edit"!
- We use the "edit" mode:
- ONLY when updating a revision with `arc diff --verbatim`.
- The only difference is in which fields may be overwritten. Under "create", all fields may be overwritten. Under "edit", only safe fields may be overwritten.
- The "Differential Revision" field currently does not render in either edit mode. This is wrong. Even though it can not be updated in the "edit" mode, it should still render in both modes. This is the only material change this revision makes.
- Without this change, when we "create" a new message from a working copy and the working copy has a "Differential Revision" field, we incorrectly discard it.
- The only field which does not render on edit modes now is "Reviewed by" (not "Reviewers"), which is correct, since we do not read the value.
Test Plan: Ran "arc diff" to create/update revisions. Ran "arc diff --verbatim" to create/update revisions with implicit edits (with D2411). Ran "arc diff --edit" to update revisions with explicit edits.
Reviewers: jungejason, btrahan
Reviewed by: jungejason
CC: vrana, aran
Differential Revision: https://secure.phabricator.com/D2412
Summary:
The highlighting is distracting according to Nick Shrock and others.
Real designer, Lee Byron, helped me with this.
It also gives us unagressive target for jumping to the source line in future.
Another feature I will probably implement is highlighting also the source of copies/moves.
I will use the right side of the left column for it.
Test Plan:
Hover copied notifier.
Hover coverage notifier.
I've also checked that this doesn't break our super-flaky old/new code JavaScript detector.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin, leebyron, schrockn
Differential Revision: https://secure.phabricator.com/D2403
Summary:
It saves some time on non-highlighting generated and other not interesting code.
The code is quite complex (300 lines methods) so I'm not sure if everything is moved correctly.
P.S. I hope that moved code detector will work...
Test Plan:
Display generated file with all whitespace, verify that it is not highlighted.
Display normal file.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1134
Differential Revision: https://secure.phabricator.com/D2358
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
Summary:
This adds support to differential fields to display warnings before a revision
gets accepted. Since lint and unit are differential fields, the code for their
warnings was moved into their respective field specification classes, so there
is only one code path for warnings (lint, unit, or custom).
Test Plan:
Select 'Accept' on a revision with lint/unit warnings and see messages appear
like they used to. Change it back to 'Comment' and they go away. Repeat with
a revision without lint/unit warnings and see no warnings appear. Checked
darkconsole to see no errors due to this.
Reviewers: jungejason, epriestley, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2363
Summary:
When choosing a verb to show with a closed differential revision, choose the
verb based on the upstream vcs, not the vcs used to create the diff, since these
are not the same thing. I also updated the documentation for the next step for
an accepted diff for the case where the local vcs and backing vcs aren't the
same (since arc land doesn't work for those).
Test Plan:
Loaded a committed diff and an accepted diff from fbcode and www to check that
they show the correct thing.
Reviewers: jungejason, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1118
Differential Revision: https://secure.phabricator.com/D2360
Summary:
I think this improves things, let me know if you have feedback.
Also addresses T840.
Test Plan: See screenshots...
Reviewers: vrana, btrahan, jungejason
Reviewed By: vrana
CC: aran, zeeg
Maniphest Tasks: T840
Differential Revision: https://secure.phabricator.com/D2357
Summary:
Before: {F10754}
After: {F10753}
Test Plan:
View revision with lint warnings and unit errors.
Click on Details.
Click on Details.
Click on Details.
Click on Details.
Reviewers: asukhachev, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2351
Summary:
Also reduce the memory usage a little bit (before increasing it again).
I use the same CSS class as for the copied code.
Test Plan: Parsed 100 diffs and checked about 10 of them - looks good.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2339
Summary: The color used for this feature is pretty important and I am bad with colors.
Test Plan:
View diff created by D2320 with some copied lines and one line changed:
{F10604, size=full}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2321
Summary:
Required for D2321.
Deprecates D2320.
Uses algorithm described at D2320#16.
Complexity of this algorithm would be `O(N)` (`N` stands for number of lines) in most cases.
The worst case is `O(A*F)` (`A` stands for number of added lines, `F` for number of colliding lines) but it should be pretty rare. Real-world example is 100 modified files with moved license block (15 lines) in each. This will require 1500*100 comparisons because the algorithm will be trying to find the longest block in each file.
Test Plan:
`arc diff --only` on commit with copied code.
More tests on standalone algorithm.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2333
Summary: Inspired by D2242.
Test Plan:
Select text in left pane.
Select text in right pane.
Select all.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2249
Summary: "Committed" is SVN-specific language, and confusing in Git and Mercurial. Use neutral language instead.
Test Plan: Inspection.
Reviewers: btrahan, Makinde, vrana, jungejason
Reviewed By: vrana
CC: aran
Maniphest Tasks: T909
Differential Revision: https://secure.phabricator.com/D2087
Summary:
This is slightly more complicated for this reason:
- We don't set `dateCommitted` for normal commits, only for markcommitted.
-- We need to add this date to old revisions now.
Test Plan:
Reparse a revision - commit date was set.
Conduit `markcommitted` - commit date was set.
Run SQL script.
Display closed revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2282
Summary:
These are explicit copies of implicitly-generated Lisk methods.
See brief discussion in rPdec8bac3a3af6065166d485db80fffa70dc2abe3.
Test Plan: Looked at a diff in Differential.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D2283
Summary: NOTE: This is starting to be too hacky.
Test Plan:
View revision with inline diffs, verify that Reply is there.
View standalone - no Reply.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2263
Summary: Also link to `D1?id=` instead of `?id=` because some IE versions linked to root in this case.
Test Plan: Click on old diff's inline comment link on large revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2260
