Allow disabling the part where you write an excuse

Summary:
Not everyone thinks it's a great feature

Task ID: #

Blame Rev:

Test Plan:
arc diff --skip-excuses

Revert Plan:

Tags:

Reviewers: akramer, epriestley, ide, mroch

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1735
Add "--revision <id>" flag to "arc land"

Summary: Allow the user to pick a revision explicitly if they think they know what they're doing. Similar to "arc amend --revision", etc.

Test Plan: Obviously compells a meta-test.

Reviewers: btrahan, fratrik

Reviewed By: fratrik

CC: aran, epriestley

Maniphest Tasks: T928

Differential Revision: https://secure.phabricator.com/D1753
Use execxLocal() for ArcanistGitAPI

Summary:
  - This is simpler and reuses more code than doing "(cd %s && ...)" every time.
  - If we have an issue like HGPLAIN, we have one place to fix it now.
  - On Windows, the construct "(cd %s && ...)" does not mean what we'd like it to.

Test Plan: Ran "arc diff" in a git repo with this change.

Reviewers: btrahan, Makinde

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1761
Enable shell completion without set aliases

Summary:
Otherwise it says after hitting Tab everytime:

> Warning:  Invalid argument supplied for foreach() in arcanist/src/workflow/shell-complete/ArcanistShellCompleteWorkflow.php on line 99

Test Plan: arc h<Tab>

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1766
Warn about wrong ID in `arc download D8O88`

Summary: It also disables `arc download F7312-rainbow`.

Test Plan:
  arc download D8O88
  arc download F7312-rainbow
  arc download F7312

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1768
Improve arc compatibility on Windows

Summary:
  - When altering the include_path(), use PATH_SEPARATOR (";" on Windows, ":" elsewhere) instead of hard-coded ":".
  - Detect missing php_curl.dll extension.
  - Use APPDATA instead of HOME for storing .arcrc (the internet implies this is correct?)
  - Don't try to do chmod() stuff on Windows; it's not critical and I don't want to figure out how it works.

Test Plan: Was able to run part of some arc commands on Windows.

Reviewers: btrahan, Makinde, Koolvin

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1756
Raise a better error from PhutilUnitTestEngine when trying to use one copy of libphutil to test another copy

Summary:
We currently raise a very confusing error when we hit this case:

  Exception: The phutil library '' has not been loaded!

Because of the trickiness of init-order stuff, it's difficult to detect this more explicitly earlier -- instead, just raise a more descriptive error.

Test Plan: Ran "arc unit" in a copy of libphutil other than the one arc loads.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T580

Differential Revision: https://secure.phabricator.com/D1760
Make "arc diff --raw -C HEAD" work correctly

Summary:
There's no reason this can't work now, we're just not quite careful enough about definitions.

NOTE: If you use a message which has a revision ID in it already, this will fail in an obtuse way when launching the update editor fails because stdin isn't a tty. I'll clean this all up after T614 is fully sorted.

Test Plan: Ran "arc diff --raw -C HEAD".

Reviewers: btrahan, yairlivne

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1765
Simplify `arc help`, add `arc help --full`

Summary:
`arc help` currently prints more than 500 lines and it is hardly usable without ##> arc.help##, ##| less##, ##| grep##, ##| wc -l## or such.
It is even worse with custom commands and options.

This diff changes `arc help` to only list commands with no description (under 100 lines).
It also adds `arc help --full` which maintains the original behavior.
It doesn't change `arc help <command>`.

NOTE: BC break on different levels.

Test Plan:
  arc help
  arc help --full
  arc help help
  arc help lint

Reviewers: epriestley

Reviewed By: epriestley

CC: nh, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1769
Make git repo discovery work on Windows

Summary: This use of "(cd ..)" is outside of the RepositoryAPI proper (it's when we're trying to figure out which VCS a directory uses) and explodes on Windows.

Test Plan: @koolvin applied this patch manually and got farther than before.

Reviewers: btrahan, Koolvin, jungejason

Reviewed By: jungejason

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1779
Fix escaping of "git log ---format" command on Windows

Summary:
On Windows, the PHP function escapeshellarg() replaces '%' with ' ' (space). This is apparently because there is no safe way to escape % inside of strings.

cmd.exe does use "^" as an escape character, so I think replacing "xyz" with "^x^y^z" might work for arbitrary strings (maybe?), or at least some subset of strings, but I don't know cmd.exe well enough to make that call without being concerned I'm introducing a security issue.

Although this patch is dumb, it's certinaly safe, and can only do something wrong if the user has environmental variables like H, P, T, or x01, in which case they're sort of asking for it.

cmd.exe also truncates output on \0, so use \1 as a delimiter instead.

Seriously it's like this thing was written in 1982 and never ever changed.

Test Plan: Created D1783 successfully after applying this patch.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1785
Improve ArcanistMercurialParser's handling of unusual branch names

Summary:
See <https://github.com/facebook/phabricator/issues/97>. Mercurial branch names may contain any characters, but we currently reject branch names with spaces.

NOTE: Mercurial allows you to name branches things like `     ` (five spaces). We do not parse this correctly. Die in a well fire.

Test Plan:
  - Added some horrible-but-possible test cases for crazy branch names.
  - Unit tests now pass.

Reviewers: btrahan, Makinde, killermonk

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1795
Catch "ConduitClientException", not "Exception", in "arc patch"

Summary: This may raise other types of exceptions, e.g. http/network exceptions, where getErrorCode() is not defined.

Test Plan: Reasoned about beahvior.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1804
Arc patch - upgrade "same base rev" check to "does this commit exist in the working copy?" check

Summary: good title

Test Plan: ran "arc patch DX" a bunch

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T892

Differential Revision: https://secure.phabricator.com/D1789
Fix arc diff / arc patch to handle files with missing trailing newlines properly

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
When a file doesn't begin "<?php", raise one warning, not one for each line

Summary: Linter flipped out on D1817; reign it in.

Test Plan: Ran "arc lint" on D1817, got one message.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1818
Add a lint warning for implicit fallthrough in switch statements

Summary: If a case does not end with break, continue, throw, exit or return and does not have a "fallthrough" comment, raise a warning.

Test Plan: Ran test case; ran linter against all of Phabricator (no hits).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1824
Use PhutilArgumentParser for some argument parsing in Arcanist

Summary:
Instead of doing custom parsing for --trace and --no-ansi, use builtin parsing.

This also gives us access to xprofile.

I eventually want to fully switch over, but that'll take some work.

Test Plan: Ran `arc list --trace`, `arc list --no-ansi`, etc.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1827
Improve performance of XHPASTLinter

Summary:
  - XHPASTLinter + coverage is ass-slow.
  - Use caching/perf options introduced by D1828.

Test Plan:
  - Ran profiling for unit tests with new --xprofile command line flag.
  - Old profile: https://secure.phabricator.com/xhprof/profile/PHID-FILE-uiuwsqa5wulj7eyfkjy2/
  - New profile: https://secure.phabricator.com/xhprof/profile/PHID-FILE-nl635t3jcp2sfo2spzwu/
  - Overall runtime decreased from 18.2s to 3.7s (4.9x performance increase) with coverage enabled.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1829
[ArcanistPhutilTestCase] add ->assertException() to the base test case

Summary: Simpler assert function for asserting a type of exception was raised.

Test Plan: Wrote this for (and tested it with) D1836.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1837