1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-15 19:32:40 +01:00
Commit graph

238 commits

Author SHA1 Message Date
epriestley
d744d5d859 Fix binary/utf8 issues with Differential changeset parse cache
Summary:
Fixes T4898. After we increased the strictness of the `%s` conversion, most `serialize()` output is rejected from the cache.

Drop the cache, change the column type to latin1_bin, and then use `%B` to mark the data as binary during query construction.

Test Plan: Viewed Differential, saw cache fills.

Reviewers: btrahan, spicyj

Reviewed By: spicyj

Subscribers: epriestley

Maniphest Tasks: T4898

Differential Revision: https://secure.phabricator.com/D9171
2014-05-17 16:34:13 -07:00
epriestley
a272ddcc78 Fix two parsing issues for diffs with damaged whitespace
Summary:
Fixes T4941. If a diff has had trailing whitespace stripped, we will fail to handle empty lines correctly (previously, these lines had a leading space when the original tool emitted them).

(This probably stopped working around the time we began retaining newlines.)

Test Plan: The diff in T4941 now parses and renders correctly.

Reviewers: asherkin, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4941

Differential Revision: https://secure.phabricator.com/D8968
2014-05-04 10:48:49 -07:00
epriestley
592591e715 Clean up various pieces of dead/obsolete Differential code
Summary:
Ref T2222.

  - Removes `DifferentialTasksAttacher`, which has had no callsites for a very long time.
  - Moves `differential.getrevisioncomments` off `DifferentialCommentQuery`.
  - Moves Releeph churn field off `DifferentialCommentQuery`.
  - Removes dead code in `DifferentialRevisionViewController`.
  - Removes `DifferentialException` (no references).
  - Removes `DifferentialRevision->loadComments()` (no callsites).
  - Removes `DifferentialRevision->loadReviewedBy()` (all callsites updated).
  - Removes `DifferentialCommentQuery` (all callsites updated).

Test Plan: Mostly a lot of `grep`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8476
2014-03-11 13:02:19 -07:00
epriestley
aff34077c5 Move Differential commit message parsing to a separate, tested class
Summary: Ref T2222. We have a hunk of logic that purely does text parsing here; separate it and get coverage on it.

Test Plan:
  - Ran new unit tests.
  - Used `differential.parsecommitmessage`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8444
2014-03-07 17:44:35 -08:00
Joshua Spence
6270114767 Various linter fixes.
Summary:
- Removed trailing newlines.
- Added newline at EOF.
- Removed leading newlines.
- Trimmed trailing whitespace.
- Spelling fix.
- Added newline at EOF

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: hach-que, chad, Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D8344
2014-02-26 12:44:58 -08:00
epriestley
92ac8b5ad9 Make Differential inline comment rendering more consistent and somewhat more modern
Summary: Ref T2222. Ref T1790. I partially modernized this recently, but bring it to the mail version too.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: zeeg, aran

Maniphest Tasks: T1790, T2222

Differential Revision: https://secure.phabricator.com/D8294
2014-02-21 11:52:03 -08:00
epriestley
13dae05193 Make most file reads policy-aware
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.

Test Plan:
  - Viewed Differential changesets.
  - Used `file.info`.
  - Used `file.download`.
  - Viewed a file.
  - Deleted a file.
  - Used `/Fnnnn` to access a file.
  - Uploaded an image, verified a thumbnail generated.
  - Created and edited a macro.
  - Added a meme.
  - Did old-school attach-a-file-to-a-task.
  - Viewed a paste.
  - Viewed a mock.
  - Embedded a mock.
  - Profiled a page.
  - Parsed a commit with image files linked to a revision with image files.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7178
2013-09-30 09:38:13 -07:00
Bob Trahan
ab2ae9e47f Differential - make sure not to return change type header if we're not top level
Summary: Followup to D6924. Fixes T3824.

Test Plan: deleted a file in a diff. was able to view file content without JS errors

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3824

Differential Revision: https://secure.phabricator.com/D6963
2013-09-12 16:00:00 -07:00
Wez Furlong
11f1268e99 don't throw BadMethodCallException on diffs that add images
Summary:
I don't know if there is something more sinister going on
under the covers, but we have a couple of diffs that trigger:

Unhandled Exception ("BadMethodCallException")
Call to a member function getMetadata() on a non-object

when the diff page is handling its async render calls.  One diff
in particular has multiple image adds and thus has a stack of of these
error dialogs to close.

This isn't a new regression, we just haven't gotten around to debugging
it until now (reported on 6/12)

One revision that triggers it has two diffs.  If I show Base -> Diff 1
I don't hit the error.  When I select Base -> Diff 2, or Diff 1 -> Diff
2, the error triggers.

I don't understand what this means, but this diff avoids the null object
reference that causes the exception.

Test Plan:
Load the offending diff, don't hit the error. The diff loads
the images that were added

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6851
2013-08-30 08:15:10 -07:00
Jakub Vrana
0529acd05b Fix typo in comment 2013-06-28 09:40:00 -07:00
epriestley
5d1f94ac8a Fix some Phabricator lint warnings
Summary: Lint.

Test Plan: Lint.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6127
2013-06-04 15:28:24 -07: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
epriestley
4f42d85c1c Use a serialize()-based cache instead of a JSON-based cache for changesets
Summary: These are full of PhutilSafeHTML objects now, which are destroyed by JSON serialization.

Test Plan: Dropped cache, then reloaded pages.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4942
2013-02-13 14:45:57 -08:00
epriestley
4bd2ad9270 Merge branch 'master' into phutil_tag
Auditors: vrana
2013-02-13 12:42:57 -08:00
vrana
4eb84149c2 Convert everything to safe HTML
Summary: Sgrepped for `"=~/</"` and manually changed every HTML.

Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4927
2013-02-13 12:35:40 -08:00
vrana
b72e7969a1 Move Pygments handling to libphutil
Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4875
2013-02-09 15:19:55 -08:00
Nick Pellegrino
3e6fa43658 getConfigEnv fails fast when key is not found and no default value is given.
Summary:
T2345
getConfig throws an Exception when the key does not exist.
Also removes dead code that throws an Exception.

Test Plan:
Reloaded the Phabricator home page.  In the process, found
2 Exceptions thrown due to nonexistent keys.  After addressing these problems,
the home page loads without Exceptions.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4541
2013-01-19 12:11:28 -08:00
vrana
df8d0a578b Correctly handle unset generated paths 2013-01-16 16:57:09 -08:00
Ricky Elrod
e974ffea7f Remove curly braces around line count.
Summary:
I'm assuming they aren't meant to be there.

Old:
This file has a very large number of changes ({4,032} lines). Show File Contents

New:
This file has a very large number of changes (4,032 lines). Show File Contents

Test Plan:
Saved a 5000-line test file, diffed from /dev/null, uploaded diff to local
instance, viewed.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4462
2013-01-16 07:26:40 -08:00
epriestley
4566b86376 Fix two diff rendering bugs
Summary:
  - The order of operations for file changes is wrong. We need to wrap them in a table, then render the changeset talbe around that table.
  - Line data was being set to late, so renderShield() got the wrong line count and rendered empty diffs behind, e.g., generated changes.

Test Plan: Looked at an image change with property changes. Clicked "show file contents" on a generated file.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4432
2013-01-15 08:07:40 -08:00
epriestley
2de107f21b Remove DifferentialChangesetRenderer->(g|s)etLineCount()
Summary: The "line count" is always the same as count($this->getOldLines()), and somewhat misleading since it's really "number of rows in the two-up view". D4421 removed the only (or only remaining?) call.

Test Plan: looked at some diffs

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4422
2013-01-14 14:20:26 -08:00
epriestley
612b29fff4 Implement basic one-up and test renderers
Summary:
This is a half-step toward one-up and test renderers. This is mostly a structural change, and has no user-facing impact. It splits the rendering hierarchy like this:

  - Renderer (more methods are abstract than before)
    - HTML Renderer (most HTML stuff has moved down from the base to here)
      - HTML 1-up (placeholder only -- not yet a functional implementation)
      - HTML 2-up (minimal changes, uses mostly old code)
    - Test Renderer (unit-testable renderer base, implements text versions of the HTML stuff)
      - Test 1-up (selects 1-up mode for test rendering)
      - Test 2-up (selects 2-up mode for test rendering)

Broadly, I'm trying to share as much code as possible by splitting rendering into more, smaller stages. Specifically, we do this:

  - Combine the various sorts of inputs (changes, context, inlines, etc.) into a single, relatively homogenous list of "primitives". This happens in the base class.
    - The primitive types are: old (diff left side), new (diff right side), context ("show more context"), no-context ("context not available") and inline (inline comment).
  - Possibly, apply a filtering/reordering step to the primitives to get them ready for 1-up rendering. This mostly removes information, and does a small amount of reordering. This also happens in the base class.
  - Pass the primitives to the actual renderer, to convert them into HTML, text, or whatever else. This happens in the leaf class.

The primitive implementation is not yet complete (it doesn't attach as much information to the primitives as it should -- stuff like coverage and copies), but covers the basics.

The existing HTMLTwoUp renderer does not use the primitive path; instead, it still goes down the old path.

Test Plan: Ran unit tests, looked at a bunch of diffs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4421
2013-01-14 14:20:06 -08:00
epriestley
dbdb01f858 Fix whitespace and unchanged file shields
Summary:
Fixes T181. I actually have no idea what the original issue in T181 was, but this fixes several problems:

  - The code for figuring out whitespace-only changes was extremely confusing and probably buggy (the code for figuring out unchanged files was equally confusing but probably less buggy).
  - When rendering a whitespace shield, we did not offer a "Show Changes" link. Instead, show the "Show Changes" link: we can always render the content beneath this link.
  - When clicking "Show Changes", we used the current whitespace mode, which might result in a diff with no changes. Instead, force "show all" whitespace mode.
  - We never offered a "show changes" link for unchanged files. Instead, offer this link if we can service it.
  - When clicking "Show Changes", we pierced the shield but didn't force file content, which would fold the entire file even if it was available. Instead, force the file content.

Test Plan: Generated whitespace-only and no-change diffs. Clicked shield links, or verified no shield link available in no-change-with-no-content. Generated an "@generated" change, verified shield worked correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T181, T2009

Differential Revision: https://secure.phabricator.com/D4407
2013-01-11 15:27:42 -08:00
epriestley
039a5a6d86 Simplify hunk parsing
Summary:
Simplify whitespace-ignoring diffing:

  - Currently, we generate two diffs (one ignoring whitespace, one normal) and then copy the //text content// from the normal one to the whitespace-ignoring one.
  - Instead, copy the //change types// from the ignoring one to the normal one.
    - This is cheaper and much simpler.
    - It means that we have the right change types in reparseHunksForSpecialAttributes(), and can simplify some other things.

Simplify whitespace changes, unchanged files, and deleted file detections:

  - Currently, we do this inline with a bunch of other stuff, in the reparse step.
  - Instead, do it separately. This is simpler.

Simplify intraline whitespace handling:

  - Currently, this is a complicated mess that makes roughly zero sense.
  - Instead, do it in reparse in a straightforward way.

Partially fix handling of files changed only by changing whitespace.
Partially fix handling of unchanged files.

Test Plan:
  - Ran unit tests.
  - Created context-less diffs, verified they rendered reasonably.
  - Generated a diff with prefix whitespace, suffix whitespace, intraline whitespace, and non-whitespace changes. Verified changes rendered correctly in "ignore most" and "show all" modes.
  - Verified unchanged files and files with only whitspace changes render with the correct masks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4398
2013-01-11 15:25:15 -08:00
epriestley
6a69108358 Simplify context parsing and add test coverage
Summary:
  - Remove 'missingNew', etc. It's impossible for a diff to popluate these, as far as I can tell (I can't generate such a diff, or find any which generate it).
  - Add unit tests.

Test Plan: Unit tests, viewed a diff with some missing context.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4393
2013-01-10 16:06:39 -08:00
epriestley
9aeebbbf58 Fix "context not available" in contextless diffs
Summary: D4351 swapped this from a map to a list, and then stopped it from getting to the ChangesetParser so it didn't make it to the Renderer.

Test Plan: Ran "arc diff", copy/pasted it into the web UI. Saw a diff with "context not available" blocks in between missing contexts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4384
2013-01-10 09:51:52 -08:00
Bob Trahan
f1d7c8fbf8 fix whitespace mode for differentialHunkParser
Summary: I forgot to "set it" from the changeset parser.

Test Plan: made a diff with silly whitespace changes. toggled the various whitespace modes and got appropriate results

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4378
2013-01-09 13:28:27 -08:00
Bob Trahan
84c27ae255 re-factor DifferentialChangesetParser pass 3 / N
Summary: introducing a new friend called DifferentialHunkParser. Sort of like the DifferentialChangesetParser but works with hunks only. tried to grab hunk parsing type things from across the code base and move them into this new class.

Test Plan: unit tests and played around in Differential a bit.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4351
2013-01-09 13:11:17 -08:00
Bob Trahan
fcc5366eff fix differential + diffusion for showing generated files
Summary: in the re-factor quest this got broken. Turns out NewLines (aka $this->new in the parser) gets updated right up until the very end for showing text changes so instead pass over the total line count to the renderer to get this codepath right.

Test Plan: showed content of some generated files in diffusion and all was well

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2204

Differential Revision: https://secure.phabricator.com/D4237
2012-12-19 10:56:00 -08:00
epriestley
406833ba90 Minor, fix a variable name after recent refactoring.
Test plan: Observed no fatal when viewing a diff which removes an image.

Auditors: btrahan
2012-12-13 09:49:03 -08:00
Bob Trahan
c970f7e89c refactor pass 2 of N on DifferentialChangesetParser and bonus bug fix
Summary: pull out some more stuff from the TwoUp renderer that's generically useful. also clean up $xhp variable and add a get method for the $coverage. Finally, fix T2177 while I'm in here.

Test Plan: played around with Differential. Also opened things up in Firefox to verify T2177.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2009, T2177

Differential Revision: https://secure.phabricator.com/D4161
2012-12-11 17:16:11 -08:00
epriestley
6ed02e6ee8 Restore "Reply" action to Differential inline comments
Summary: Minor issue from D4117, user doesn't get passed down so inline comments are missing their "reply" link.

Test Plan: Looked at Differential, saw reply link.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4145
2012-12-10 15:12:32 -08:00
hfcorriez
81d59a0b77 Fix call to undefined method for DifferentialChangesetParser::originalRight() 2012-12-10 17:47:58 +08:00
Bob Trahan
638449b483 removed some debugging code from D4117
Summary: whoops!

Test Plan: no more debugging code

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4121
2012-12-07 18:08:27 -08:00
Bob Trahan
75e8ff26f5 Refactor DifferentialChangesetParser -- pass 1 of N
Summary:
basically did my darnedest to pull out a TwoUp rendering view. Made a base class for the rendering views with "old" and "new" terminology rather than "left" and "right.

Future revisions will finish cleaning up the terminology within the DifferentialChangesetParser itself and more of the ideas within T2009.

Test Plan: been playing with differential all day

Reviewers: epriestley

Reviewed By: epriestley

CC: vrana, chad, aran, Korvin

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4117
2012-12-07 16:19:57 -08:00
Bob Trahan
08172cf361 fluid diff -- more table silliness
Summary: inline comments eat up all 3 tds and no code coverage should be shown.

Test Plan: verified in FIREFOX that inline comments looked good

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2005

Differential Revision: https://secure.phabricator.com/D4115
2012-12-07 15:53:24 -08:00
Bob Trahan
8a6e82eba6 more fixes to fluid diff
Summary: always render "copy" column for text. this gives us consistent left alignment. also tweak css to be less wide for the copy and code coverage columns.

Test Plan: http://phabricator.dev/rP1a3bf098ae4ab4f8add4af744a6b93a257851fb0 looks even better

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2005

Differential Revision: https://secure.phabricator.com/D4110
2012-12-07 12:37:11 -08:00
Bob Trahan
1922590f98 fix fluid diff change
Summary: when we had a change that had new data and uncommitted changes the colspan could get off. make sure to only decrement the colspan if we are actually on a new line

Test Plan: http://phabricator.dev/rP1a3bf098ae4ab4f8add4af744a6b93a257851fb0 now looks good on Firefox

Reviewers: vrana, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2005

Differential Revision: https://secure.phabricator.com/D4109
2012-12-07 12:00:28 -08:00
epriestley
210f8e0390 Fix leftover call to renderRightCode() from D4083
See T2165.

Auditors: btrahan
2012-12-07 08:16:47 -08:00
Bob Trahan
b4de56ef4b make differential use fluid width for code layout
Summary:
assume at least 360px for a given code pane. that's about when the comment box starts fighting back anyway. we'll use the yet-to-be-built one page render for the narrow viewport cases.

This address the cases as laid out in T2005. It fails the "MMMMM" case pretty horribly. However, if there is a space it works just fine and presumably folks are stretching out their windows on big glorious monitors at 160 characters wide or whatever.

Re-factored things just a tad but figure I'll take a nice big chunk of "renderer" to move forward T2009

Test Plan: looked at all sorts of funky diffs

Reviewers: epriestley

Reviewed By: epriestley

CC: chad, aran, Korvin

Maniphest Tasks: T2005

Differential Revision: https://secure.phabricator.com/D4083
2012-12-06 11:33:04 -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
Bob Trahan
e0b9a63388 make Differential comment panel flexible width and nip a bit at other flexible width issues
Summary:
all sorts of stuff

 - made comment form width flexible
 - made margins element specific rather than part of differential-primary-pane
 - made box elements all veritically align left and right until code stuff
 - re-factored width calculaton stuff a bunch so only the code section has to suffer from max-width calculations; everything else can flex
 - made colspan 3 for rightmost table header element. this is so the "View Options" UI element ends up lining up correctly with the "Show All Lines" element just below

Test Plan: looked at revision view and changeset view and it all looked hot. note I did not test what things looked like with different word wrap values; that should still work given the re-factoring and not re-design here. also toggled haunted panel mode and it looked good.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2006

Differential Revision: https://secure.phabricator.com/D3866
2012-11-01 13:30:37 -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
epriestley
c0b9b6db54 Don't count "\r" or "\n" for purposes of linewrapping text in Differential
Summary: This should all go away at some point when we move to fluid layout, but don't be more annoying than necessary in the meantime.

Test Plan: Meta.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3788
2012-10-22 16:24:22 -07:00
epriestley
67498f3334 Restore phutil_split_lines() to Differential
Summary:
Same as D3759 with a fix:

Previously, we would insert `null` to indicate a line that doesn't exist on one side of a diff, and then implode on "\n", so we'd get "\n" as a result.

In D3759, we'd insert `null` but implode on empty string, and get nothing as a result.

When a placeholder null is present, explicitly insert a newline.

Test Plan: D3759 plus examined a diff with removed lines on the left side prior to new lines on the right side.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3765
2012-10-20 07:24:27 -07:00
epriestley
468aeaef0d Revert "Use phutil_split_lines() in Differential"
This reverts commit f6cb51562e.

This has some bugs in normal diffs that I haven't immediately been able to figure out. I'll reapply it once I sort them out.

Auditors: vrana
2012-10-20 06:42:08 -07:00
epriestley
f6cb51562e Use phutil_split_lines() in Differential
Summary:
  - We currently treat "\r" as a newline, but should not because VCSes do not.
  - We get an extra empty line at the end of diffs created after D3442 because we now retain newlines.
  - Historically we've converted tab pre-cache, but do it post-cache instead so we can add prefs about it, as we should handle it better than we do (e.g., let the user set it to a different width, infer width from comments in the file, expand it to actual tab stops, or show it visually in some way).

Test Plan:
  - Verified diffs no longer have an empty line at the end.
  - Created a diff of a "\r" file and verified it displayed somewhat reasonably. All browsers treat "\r" as a real newline so it's not necessarily perfect, but we can clean that up later. Hopefully these files are exceedingly rare.
  - Created a file with tabs and verified it came out reasonably.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1857

Differential Revision: https://secure.phabricator.com/D3759
2012-10-20 06:13:24 -07:00
Wez Furlong
1b6eced757 avoid claiming that the source of a file move was unchanged
Summary:
sometimes we show moved files as unchanged, sometimes deleted,
and sometimes inconsistent with what actually happened at the
destination of the move.  Rather than make a false claim,
omit the 'not changed' notice if this is the source of a move.

Background: one of our users was confused by the source of a move
claiming to be unmodified when the destination of the move had
extensive modifications.

There may be a question about the quality of the data we keep/compute
about the changes, so maybe we could do a better or more consistent
job there (may just be nuances of the SCM in use); this approach
just smoothes that out and doesn't require fixing up the status
in pre-existing diffs.

Test Plan:
In the facebook phabricator, D594195#739ace1a and D590020#b97cfcfd
For others, find a diff that has moved files.
For the source of a move, we should now only show the "X moved to Y"
header and not the "unchanged" shield.

Reviewers: nh, vrana, epriestley

Reviewed By: vrana

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3669
2012-10-09 14:16:21 -07:00
vrana
58206a146f Fix displaying gap context when showing first lines
Summary: Show First 20 Lines doesn't display gap context and emits error.

Test Plan: Showed first 20 lines, last 20 lines, middle 20 lines - saw context and no error.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3616
2012-10-04 09:07:47 -07:00
vrana
377a29644f Display shield for generated binary files
Test Plan:
Displayed diff with generated images.

Should hide [[ https://secure.phabricator.com/D3579?vs=on&id=7076&whitespace=show-all#bfb5f5e5 | webroot/rsrc/image/autosprite.png ]] after deploy.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3585
2012-10-01 15:18:43 -07:00
vrana
958e6cd109 Add missing break statement 2012-09-06 15:12:57 -07:00
vrana
0f9f8b5f30 Tidy context displayed in gap
Summary:
We have some complaints on this feature:

- It's not clear that the displayed code is context from gap.
- It would be better if the context would be displayed with its real indentation.
- It's not clear how far the context is from the displayed code.
- Links revealing gap aren't on consistent place.

This solves all these problems and introduces new one:
It now seems that the reveal links works only with the left side.
Anyway, I think that this is better overall.

I don't want to put the context on a separate line to not waste space.

Test Plan: Displayed various contexts, revealed context.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, bh, jwatzman

Differential Revision: https://secure.phabricator.com/D3404
2012-08-29 17:25:17 -07:00
vrana
8b715a64b5 Don't waste too much cache by a single changeset
Summary:
We have `max_allowed_packet` 1 GiB but our replication dies if the query is longer than unknown value (it dies with 293 MB long query).

Anyway, there's no reason why we should not save the cache if you have small `max_allowed_packet`.

Test Plan: Lowered `$size` to 100, deleted cache from DB, displayed changeset, verified issued queries in DarkConsole, verified DB.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3390
2012-08-27 16:51:05 -07:00
Alan Huang
5a0b640ccd Bump cache version
Summary:
See T1602#15.

I don't intend to land this right away, because I'm not sure I won't
need another change or two to the rendering code. Keeping it here so I
won't forget about it.

Test Plan: iiam

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1602

Differential Revision: https://secure.phabricator.com/D3240
2012-08-23 16:16:19 -07:00
vrana
ca1775b468 Display context in changeset gaps
Summary:
I quite often wonder what's inside these gaps displayed in changeset. In which method I am? Is it a while loop or a foreach? Maybe I'm in a class but the project doesn't have a sctrict policy of one class per file so what's the name of the class?

I've experimented with bunch of rules:

- Always display 0 indentation: useless for one class per file.
- Always display 1 indentation: weird inside global functions.
- Display closest lower indentation: works best.

I'm not sure about highlighting the context. I like highlighting but maybe in this case subtler monochrome text will work better.

Test Plan: Browsed bunch of diffs, loved it.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3371
2012-08-23 08:57:52 -07:00
vrana
ea5d9d40e1 Delete empty command 2012-08-22 22:54:00 -07:00
Alan Huang
6fc01aa5fd Change background for image views in Differential
Summary:
Many images in Differential changesets are icons designed for
use on dark backgrounds. This makes them invisible on Differential's
white background. This adds an option to use a darker background
instead so you can see the images.

Currently this behavior is triggered on hover. (Also, it's a rather
garish fuchsia.) It seems fine UX-wise but I'm not totally sure of it.

Test Plan:
Load diff containing grippy_texture. Marvel at the grippy
fuchsia.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3272
2012-08-13 17:21:16 -07:00
vrana
251438b2c2 Translate change type header
Summary: This is a tax for internationalization.

Test Plan: Displayed a revision with added and moved files.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3166
2012-08-06 12:07:26 -07:00
vrana
aefc5654c3 Load contents of shielded files with lots of changes
Summary:
We need `$this->old` and `$this->new` in `renderShield()`.

Broken since D2358.

Test Plan: Loaded contents of file with lots of changes.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3080
2012-07-26 13:28:10 -07:00
vrana
00160ab2a3 Remove ambiguous test 2012-07-18 18:31:34 -07:00
vrana
273211ae3e Highlight also lines that were added on the other side in diff of diffs 2012-06-29 18:19:01 -07:00
vrana
a2f4d661b9 Fix computing line numbers in diff of diffs
Test Plan: Live in prod.

Auditors: jungejason
2012-06-29 18:03:26 -07:00
vrana
99df72b81c Unhighlight lines modified in rebase
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
2012-06-29 17:21:23 -07:00
vrana
bf10eaf81c Wrap Differential property changes 2012-06-19 14:15:40 -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
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
6f10706852 Display and link lint errors on line 0
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
2012-05-29 16:53:30 -07:00
vrana
5a1d89227b Use subtler highlighting for copied and moved code
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
2012-05-10 10:25:24 -07:00
epriestley
b80ea9e0e8 Revert cd63d9b2ce / D2403, see diff. This broke inline comments by making them not span enough columns. 2012-05-07 06:01:56 -07:00
vrana
cd63d9b2ce Use subtler highlighting for copied and moved code
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
2012-05-04 21:56:24 -07:00
vrana
3fa310f3c0 Render shields before highlighting code
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
2012-05-04 12:43:46 -07:00
vrana
e08b4cbb2c Inform about moved code and prefer it over copied code
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
2012-04-30 11:01:15 -07:00
vrana
13a48a79d7 Highlight copied/moved lines in Differential
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
2012-04-28 21:46:31 -07:00
vrana
0732f2f9dd Hide selection to provide a visual cue about clipboard JS magic in Differential
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
2012-04-24 18:53:48 -07:00
vrana
7e571994bc Don't display inline comment's Reply in standalone view
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
2012-04-18 10:44:24 -07:00
vrana
752d742085 Fix copy/paste error from D2227
Auditors: epriestley
2012-04-17 16:24:49 -07:00
vrana
74cdad29d0 Don't save highlithing errors to cache
Summary:
If I have Pygments enabled in config but `pygmentize` doesn't work then unhighlighted source is stored to cache.
If I later make `pygmentize` work then the unhighlighted source is still loaded from the cache.

Test Plan:
Break `pygmentize`.
View a diff with JS files.
Fix `pygmenize`.
View the diff again.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, gatos99, Koolvin

Differential Revision: https://secure.phabricator.com/D2227
2012-04-17 12:48:27 -07:00
vrana
974b576df0 Fix whitespace 2012-04-09 16:57:17 -07:00
vrana
bc61f36beb Replace elseif by else if
Summary:
Mostly written by me.
Omit external libraries.

Test Plan: http://phabricator.com/docs/phabricator/article/PHP_Coding_Standards.html

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2104
2012-04-04 15:24:47 -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
epriestley
315870d56a Fix various newline problems in the difference engines
Summary: I'll mark this one up inline since it's all separate bugs.

Test Plan:
  - Created a diff with eight changes: (newline absent -> newline present, newline present -> newline absent, newline present -> newline present, newline absent -> newline absent) x (short file with change near end, long file with change near middle).
  - Viewed diff in Ignore All, Ignore Most, Ignore Trailing and Show All whitespace modes.
  - All 32 results seemed sensible.
  - Really wish this stuff was better factored and testable. Need to fix it. :(

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T1030

Differential Revision: https://secure.phabricator.com/D1992
2012-03-22 14:13:48 -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
57fd4bc68b Kind-of-terrible (?) oncopy handler
Summary: Works in Safari, Firefox, Chrome.

Test Plan: Copied some text, threw up a little in my mouth.

Reviewers: aran, tuomaspelkonen, tomo, rstout, btrahan

Reviewed By: aran

CC: aran, epriestley, ddfisher

Maniphest Tasks: T145, T995

Differential Revision: https://secure.phabricator.com/D244
2012-03-15 19:04:59 -07:00
vrana
d8bb59bd9a Don't display old lines on right side in diff
Summary: I've done the same stupid copy/paste mistake twice in one day :-(.

Test Plan: Display diff with no newline at end of file in new file.

Reviewers: davidreuss, epriestley

Reviewed By: davidreuss

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1911
2012-03-15 05:45:00 -07:00
vrana
0c20d7900e Highlight files with no newline at end of file
Summary: Text "No newline at end of file" was sent to highlighter causing error with most languages.

Test Plan: Display diff containing file with no newline at end of file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1903
2012-03-14 18:31:35 -07:00
vrana
1464c0f2eb Unhighlight " No newline at end of file" in Differential
Summary:
It looks like there is really this text written e.g. at https://secure.phabricator.com/D1896#0a6a1957

I am not sure that it is the only place which needs to be fixed.

Test Plan: Display diff with no newline at end of file in Differential.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1900
2012-03-14 17:58:14 -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
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
392d12f1fc When marking a line changed because of internal whitespace in "Ignore Most", intraline diff it
Summary:
Currently, we mark some lines that otherwise count as "unchanged" to be "changed" when they have internal whitespace changes.

However, we've already excluded them from the intra-line diff algorithm. Unset the flag so they can get intra-line diffed.

Test Plan: Viewed a change with internal whitespace changes, internal whitespace changes were intraline diffed.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T970

Differential Revision: https://secure.phabricator.com/D1871
2012-03-12 18:20:51 -07:00
epriestley
f6fda5ec01 Minor, address feedback frmo D1864. 2012-03-12 17:10:05 -07:00
epriestley
85bdcbdd43 Show coverage percentages in table of contents
Summary:
Rough cut -- this needs style / color / tooltips, etc. Show raw coverage and "modified coverage" (coverage on lines you touched) in the table of contents.

https://secure.phabricator.com/file/data/id3apce5p5gevkee6tg2/PHID-FILE-kxcxlbsej454t4xiht2o/Screen_Shot_2012-03-12_at_3.30.30_PM.png

Test Plan: See screenshot above.

Reviewers: tuomaspelkonen, btrahan, zeeg

Reviewed By: tuomaspelkonen

CC: aran, epriestley

Maniphest Tasks: T965

Differential Revision: https://secure.phabricator.com/D1864
2012-03-12 17:06:55 -07:00
epriestley
09c1bd34f1 Add an "ignore all" whitespace mode
Summary:
We used to have "ignore all", but it became "ignore most". It does not ignore in-line whitespace changes or whitespace changes in language where whitespace is marked semantic.

Add an explicit "ignore all" option.

Test Plan: Used "ignore all" to view a change with heuristically-detected semantic whitespace, the change was ignored.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T970

Differential Revision: https://secure.phabricator.com/D1865
2012-03-12 17:06:36 -07:00
epriestley
8dfe8e84f0 Improve Diffusion parsing of submoudule changes
Summary: We currently parse these as directory changes and discard them. Instead, parse them as a new "SUBMODULE" type of change.

Test Plan:
  - Reparsed a commit which changes submodules and verified it parses correctly.
  - Reparsed a commit which adds submodules and verified it parses correctly.

Reviewers: btrahan, kdeggelman

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1815
2012-03-07 14:24:43 -08:00
vrana
6cb5db60d5 Display compatible inline comments (usually synthetic) on the same row
Summary:
This just looks silly:
{F8088, size=full}

It runs in O(N*N) but it's not a big deal because there are usually only few
comments per line.
I didn't implement it for images.

Test Plan:
View revision with compatible inline comments in two diffs.
View revision with different inline comments on same line in two diffs.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1570
2012-02-04 10:20:37 -08:00
epriestley
29337a9b3a Minor, add "cov" class to inline comment <td /> so new files with lint in them do not go crazy. 2012-01-31 18:30:22 -08:00
epriestley
1e9492f8c2 Show coverage information in Differential
Summary:
Render coverage information in the right gutter, if available.

We could render some kind of summary report deal too but this seems like a good
start.

Test Plan:
  - Looked at diffs with coverage.
  - Looked at diffs without coverage.
  - Used inline comments, diff-of-diff, "show more", "show entire file", "show
generated file", "undo". Nothing seemed disrupted by the addition of a 5th
column.

Reviewers: btrahan, tuomaspelkonen, jungejason

Reviewed By: btrahan

CC: zeeg, aran, epriestley

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D1527
2012-01-31 12:07:47 -08:00
Dave Ingram
3edf60627d Add support for marking files as "generated" by regexp against path
Summary:
Not all auto-generated files can include the magical
"generated" annotation for one reason or another, but they may follow
path rules. This patch allows files to be marked as automatically
generated by matching the path with a regular expression.

Test Plan:
Alter 'differential.generated-paths' setting in config.
Create a new diff that affects a file matching one of those regular
expressions. Verify that Differential marks it as automatically
generated and therefore probably not worth reviewing (in the same way as
the magical "generated" annotation.

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D1455
2012-01-19 18:30:19 +00:00
epriestley
ef768f9694 Use phutil_utf8_hard_wrap() in Phabricator
Summary: See D1433.

Test Plan: Created a new diff with a line >80chars, observed it wrapping
correctly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D1438
2012-01-17 09:48:58 -08:00
epriestley
05ee317555 Remove "parsedHunk" property
Summary: This is never read anywhere and clearly has no effect.

Test Plan: grep

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D1434
2012-01-17 08:09:56 -08:00
epriestley
8f1c7dc663 Remove some unused nonsense
Summary: These blocks do nothing. end() produces a side effect on the internal
array pointer, but the code does not depend on it.

Test Plan: Reasoned about the code? Also viewed some diffs.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D1432
2012-01-17 08:09:45 -08:00