Merge pull request #47 from aurelijus/master

JSHint linter issue on windows
Add mysql and mysqli functions to the phutil map whitelist

Summary: I originally excluded `mysql` these under the theory that it would be impossible to test anything without it, but `arc` doesn't need it and you could have only one of the mysql-flavored extensions without having the other one. Whitelist all the mysql and mysqli extension functions as far as depenencies are concerned.

Test Plan: @floatinglomas, let me know if this fixes things for you?

Reviewers: btrahan, vrana, floatinglomas

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3119
Add an arc flag workflow

Summary:
Allow querying and modifying flags from arcanist. Currently
supports only printing and deleting flags for Differential revisions,
but it should be straightforward to add more capabilities (given Conduit
support).

Test Plan:
Run arc flag, passing it various revisions. Flags are
modified appropriately.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1556

Differential Revision: https://secure.phabricator.com/D3133
Use PhutilConsole in lint and unit

Test Plan:
`arc lint` with OK result and with patchable error.
`arc unit` with passes and errors.
`arc diff` with patchable lint error.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3151
Throw properly if there are uncommitted changes under Git

Summary: This was accidentally disabled with some Mercurial changes that allowed dirty working copies.

Test Plan: Ran `arc diff` with staged changes.

Reviewers: nh

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D3165
Fix an issue where branches at the same commit race to destruction in "arc branch"

Summary:
If two branches have the same HEAD, they currently race to overwrite each other in $commit_map.

We don't need to return a map indexed by commit since nothing ever reads the keys out of it. Just update $branches in place.

Test Plan: With two branches at the same HEAD, ran "arc branch". Saw both branches in output.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3188
Depend on events when attaching Diff ID to postponed unit tests

Summary:
According to @epriestley, it's nasty and kind of crazy: D2933#1.
It also stands in my way for D2614.

Test Plan: Rewrote our callsite to event listener and verified that it still works.

Reviewers: tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D3171
Run lint and unit tests in `arc diff` on background

Summary:
I usually write commit messages 1-2 minutes.
`arc lint` in our repository usually runs for around 30 seconds, `arc unit` another minute or two.
Even Phabricator unit tests sometimes runs long (because `CREATE DATABASE` and `DROP DATABASE` is slow in our setup for some reason).

Waiting for the results is boring and unnecessary.

This diff presents two different concepts how to run them on background:

# Lint is run with `--output json`, results are parsed and presented back to user. It isn't perfect - there's no context in printed lint errors which is a serious problem.
# Unit tests are run normally and the results are written to a scratch file. It also isn't perfect - colors are lost during the process.

I'll probably choose one approach and use it on both places. Let me know your thoughts about them.

This can be further improved to resolve the futures also after inputting the update message but it can be done in a separate diff.

Test Plan:
- Remove lint engine.
- Remove unit engine.
- Make lint errors.
- Make unit errors.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, beng, tuomaspelkonen, alanh

Differential Revision: https://secure.phabricator.com/D2614
Run lint and unit before updating revision

Summary: Depends on D2614.

Test Plan:
Updated a diff with no lint errors.
Updated a diff with lint errors, verified that the previous message is not lost.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3174
Fix an issue where 'arc alias' reverses parameters

Summary:
We shove alias parameters onto the front of the arg list so if you make an alias like "qdiff" = "diff x y z" and then run "qdiff a b c", we end up with "diff x y z a b c". However, currently we reverse alias parameters, so you actually get "diff z y x a b c".

This is a problem for `arc alias bdiff -- diff --background 1`, which evaluates to `arc diff 1 --background` and fails.

Test Plan: Created a `bdiff` alias and ran it successfully.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3196
Redirect output to console in background mode of `arc diff`

Summary: See D3208.

Test Plan:
  $ arc diff --background 1

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3210
Allow specifying root in `arc inlines`

Summary: Simplifies editor bindings.

Test Plan:
  $ arc inlines --root W:/devtools/phabricator/

Reviewers: epriestley

Reviewed By: epriestley

CC: vii, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3229
Improve "arc patch" error messages for mismatched local and diff

Summary: We emit a confusing error if there's no ".arcconfig" in the local right now.

Test Plan:
  $ arc patch D3185

    This patch is for the 'phabricator' project,  but the working copy
    belongs to the 'phabricatox' project. Still try to apply the patch? [Y/n]

  $ arc patch D3185

    This patch is for the 'phabricator' project, but the working copy does
    not have an '.arcconfig' file to identify which project it belongs to.
    Still try to apply the patch? [Y/n]

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3231
Make the "this is technical documentation" message of "Arcanist Overview" more clear

Summary:
The only purpose of "Arcanist Overview" is to tell people they shouldn't be here, but we bury the lede.

Make it clear that this is not user documentation.

After T988 we can improve the organization here, but some recent users found this pretty confusing.

Test Plan: Generated and read documentation.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3235
Remove `beginRedirectOut()` from Arcanist

Summary: We always do this on --recon now, see D3213.

Test Plan: Ran `arc diff --background 1` to generate this diff.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3241
Fix doc links

Summary: I will also commit fixes in other repos.

Test Plan: LinkChecker

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3242
Allow arcanist to search for libphutil in externals/includes/

Summary:
In some cases (notably, homebrew) an installer may not control where arcanist/ and libphutil/ live and may not be able to control 'include_path'.

Allow libphutil/ to be symlinked into arcanist/externals/includes/ if all else fails.

Test Plan:
  - Moved `libphutil` to `libphutilx`. Ran "arc" and got a failure.
  - Symlinked it into externals/includes/, ran `arc`, got success.
  - Moved it back to `libphutil`, ran `arc`, success.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, tfmeusburger

Differential Revision: https://secure.phabricator.com/D3243
Fix serious problems with 'arc anoid'

Summary: https://github.com/facebook/arcanist/issues/48

Test Plan: hmm

Reviewers: alanh, tuomaspelkonen

Reviewed By: tuomaspelkonen

CC: aran

Differential Revision: https://secure.phabricator.com/D3244
Raise exception instead of returning magical number from 'arc anoid'

Summary: how do I python

Test Plan: help

Reviewers: alanh

Reviewed By: alanh

CC: aran

Differential Revision: https://secure.phabricator.com/D3245
Switch arcanist to phutil_utf8_convert()

Summary: See D3252. Reduces code duplication a little bit. Also remove some dire warnings about impending doom -- this has been in use in the wild for a long time.

Test Plan: Added a file in ISO-8859-1, ran `arc diff --encoding ISO-8859-1` to generate this revision, got an encoding note in output.

Reviewers: davidreuss, vrana, btrahan

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T452

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