Summary:
- When users pipe in colorized diffs, strip the colors instead of failing.
- When showing context, show line numbers (we do show 3 lines around the failure, the failure was just on the first line).
- Remove an irrelevant TODO comment (we handle this elsewhere now).
Test Plan: Unit tests.
Reviewers: Makinde, btrahan
Reviewed By: Makinde
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1887
Summary:
`svn rm $directory` removes the directory and its contents in svn 1.7, whereas
svn 1.6 only removes the directory's contents, so arc diff needs to not try
to read the directory.
Test Plan:
ran arc diff in a svn working copy that had a directory removed (with both
svn 1.6 and svn 1.7) and confirmed that the command did not throw an exception,
and that the removed directory was marked as directory (not a file).
Reviewers: epriestley, jungejason
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1889
Summary: svn changed the format of property changes for svn1.7.
Test Plan: arc diff on a working copy with svn property changes
Reviewers: epriestley, jungejason
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1885
Summary:
- When you try to "arc patch" a change which adds or removes a trailing newline or alters a file with a trailing newline near the newline, the patch fails because we fail to reconstuct the "\ No newline at end of file" line.
- We don't currently record enough information about these diffs to reconstruct them with a sensible amount of effort. Store the "\ No newline at end of file" line instead of just a flag.
- There are some special rules for these lines; implement them.
NOTE: This causes some display glitching in Differential, but nothing major; I'll fix that in a followup patch.
Test Plan:
- Created a diff which added a trailing newline, removed a trailing newline, and altered a file without a trailing newline near the end of the file.
- Verified that the git version of this patch and the 'arc export --git' version of this patch are (essentially) identical.
- Verified that "arc diff" + "arc patch" can apply these changes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T533
Differential Revision: https://secure.phabricator.com/D1810
Summary: This is probably just the first step down a long road of not handling
exotic filenames correctly, but if you diff a file named "∆.jpg" it currently
chokes while parsing the diff.
Test Plan: Added minimal failing unit test, fixed parser, test passed.
Reviewers: zeeg, btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D1592
Summary:
Mark all applicable Arcanist classes as "final", except PhutilLintEngine, which
needs a little finesse.
@jungejason / @nh, does this break any Facebook stuff?
Test Plan: Linter no longer raises warnings. Ran "testEverythingImplemented" in
Phabricator.
Reviewers: btrahan, jungejason, nh
Reviewed By: btrahan
CC: aran, epriestley
Maniphest Tasks: T795
Differential Revision: https://secure.phabricator.com/D1519
removed
Summary: Mercurial output diverges from git output when binary files are
removed. Parse the Mercurial flavor.
Test Plan:
- Unit test fails before this change and passes afterward.
- Removed a binary file in my hg local and successfully "arc diff --only"'d
the change.
NOTE: Git can't apply these patches, and reports:
$ git apply < ~/Desktop/patch.txt
error: removal patch leaves file contents
error: level.png: patch does not apply
Reviewers: Makinde, btrahan, jungejason, nh, tuomaspelkonen, aran
Reviewed By: jungejason
CC: aran, jungejason, Makinde
Differential Revision: 1126
Test Plan:
Tried viewing a diff detected as binary in diffusion with and without
setting my desired encoding. Worked as expected.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1008
mercurial changesets
Summary: Makes the parsing slightly more liberal so a test case from @Makinde
passes.
Test Plan: Ran unit tests.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1054
Summary:
See T584. Git renders these types of changes somewhat unusually. Ensure they are
rendered as changes.
The fact that a symlink was replaced is not explicitly rendered (e.g., "This
path was changed from a symlink to a regular file."), but can be inferred from
the 'unix:filemode' property change that will accompany the changeset. We could
also make it explicit but this type of change is rare enough that it's probably
good enough as-is.
Test Plan:
- Ran unit test, which previously failed and now passes.
- Created a diff which replaces a file with a symlink, verified it rendered a
little more sensibly.
Reviewers: aravindn, dschleimer, jungejason, nh, tuomaspelkonen
Reviewed By: nh
CC: aran, goldshlager, nh
Differential Revision: 1030
changes which add or alter binary files
Summary: See test cases, derived from "hg diff --git" and "git diff --binary".
Test Plan: Test cases work now.
Reviewers: Makinde, jungejason, nh, tuomaspelkonen, aran
Reviewed By: Makinde
CC: aran, Makinde
Differential Revision: 1047
Test Plan:
Go to /differential/diff/create and try to upload a patch that was created with
`git diff` when diff.mnemonicprefix was set to true.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: 1022
Summary: A minor bug in the parser prevented it from handling git diffs where an
empty file appears at the end of the diff. Since git appends an extra newline,
we failed to jump into the '$line === null' block.
Test Plan: Ran unit tests. Generated a revision which only deleted an empty
file.
Reviewers: jungejason, nh, tuomaspelkonen, aran
Reviewed By: nh
CC: aran, nh, epriestley
Differential Revision: 998
Summary:
A better definition of "binary" is "not utf-8", instead of "has some characters
not in this arbitrary regexp". Principally, this makes files with windows
newlines not autodetect as binary.
This might fix some of the issues in T365.
Test Plan: @egillth applied this patch and verified that Diffusion now shows
file content instead of detecting everything as binary in his repo full of
Windows newlines.
Reviewed By: jungejason
Reviewers: egillth, tuomaspelkonen, jungejason, aran
CC: aran, jungejason
Differential Revision: 799
Summary:
There's a lot of ground left to cover but this makes "arc diff" work (on one
trivial diff) in my sandbox, at least, and supports parsing of Mercurial native
diffs (which are unified + a custom header). Piles of missing features, still.
Some of this is blocked by me not understanding the mercurial model well yet.
This is also a really good opportunity for cleanup (especially, reducing the
level of "instanceof" in the diff workflow), I'll try to do a bunch of that in
followup diffs.
Test Plan: Ran "arc diff" in a mercurial repository, got a diff out of it.
Reviewed By: aran
Reviewers: Makinde, jungejason, tuomaspelkonen, aran, codeblock
CC: aran, epriestley, codeblock, fratrik
Differential Revision: 792
Summary:
See task. This is a bizarre edge case which we incorrectly reject but should do
our best with.
Test Plan:
Created an added copy of a file and diffed it successfully.
Reviewed By: alex
Reviewers: alex, jungejason, tuomaspelkonen, aran
CC: aran, alex
Differential Revision: 547
svn.
Summary: when you "svn cp" with a revision, we end up with some path collision
problems. This isn't a complete fix but it gets the change to Differential,
at least.
Test Plan:
Reviewers:
CC:
