1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-04 20:52:42 +01:00
Commit graph

1605 commits

Author SHA1 Message Date
Brandon Pedersen
4038e7be0a Fix commit message format returned by getCommitMessage
Git commit messages must have 2 newlines between the subject and body
If used to rewrite a commit message then the commit message will be
incorrectly reformatted.
2012-09-06 08:56:44 -06:00
epriestley
bc96d6036e Implement the arcanist top-level log handling in terms of $console->writeLog()
Summary:
  - See D3422.
  - Also improve some event configuration/debugging stuff.

Test Plan: Ran `arc list --trace`, set bogus/valid event handlers.

Reviewers: vrana, nh

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3423
2012-09-05 11:45:54 -07:00
vrana
a309e5e1eb Use punctuation in spelling linter
Summary: Also move dependency one directory down.

Test Plan:
  $ arc lint # on file with "teh"

Reviewers: jack, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3436
2012-09-05 10:14:29 -07:00
vrana
466910c700 Delete obsolete comment
Summary:
HPHP now autoloads in `is_subclass_of()`.
I've left the `class_exists()` check as the code is more explicit.

Test Plan:
  // under HPHP
  function __autoload($c) {
    echo "$c\n";
  }
  is_subclass_of('C', 'stdClass');

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3435
2012-09-04 23:13:43 -07:00
epriestley
a802a90123 Always include 'workflow' on Arcanist events
Summary:
This object is highly useful in many client event handlers (particularly for access to the CLI) and allows them to be implemented less intrusively.

This also slightly reduces code duplication.

Test Plan: Ran "arc diff".

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1753

Differential Revision: https://secure.phabricator.com/D3421
2012-09-04 12:08:04 -07:00
katherine
b0cfe9d94d [hg - arc patch] make mercurial bookmark with arc patch
Summary:
w00t!

Created new functions getBookmarkName, createBookmark that use mercurial
commands to create a new bookmark and apply the commit message when
running arc patch. Like with git, it checks if the bookmark already
exists. If it does, creates arcpatch-DXXX-1, -2 etc

Updated the mercurial section of run() to include applying the commit
message.

Pretty new to programming in general still, so I wasn't sure if I should
have just modified getBranchName and createBranch to work with Mercurial
(instead of creating new functions). This was easier for me to make sure
I didn't break the git functionality. Let me know I can merge the
functions.

Test Plan:
Tested in my www-hg repository. Probably need to test further (suggestions?)

Ran the following trials with arc patch D539987
1) the bookmark arcpatch-D539987 already existed
2) the bookmark did not exist
3) tried an invalid commit, arc patch R539987
4) --nobookmark flag (bookmark was not created, but the commit applied)
5) --nocommit flag (boomark was created, and the commit was not applied)
6) --nocommit and --nobokmark

Here's the summary of the results:
https://secure.phabricator.com/P493

Reviewers: dschleimer

Reviewed By: dschleimer

CC: aran, epriestley, phleet, bos, csilvers

Differential Revision: https://secure.phabricator.com/D3334
2012-08-31 17:05:46 -07:00
epriestley
d79664a30d Add a willCreateRevision event
Summary:
Adds an event prior to creation of a new revision so installs can muck around with titles, etc.

I'll also update the docs.

Test Plan: Ran "arc diff --trace" and observed event dispatch. Added `var_dump()` and verified $revision is a reasonable object.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, geoffberger

Differential Revision: https://secure.phabricator.com/D3408
2012-08-30 12:28:35 -07:00
Nick Harper
4a786f48de Use paste.query for arc paste
Summary: paste.info is deprecated; switch to using paste.query

Test Plan:
  arc paste P123
  arc paste P123 --json

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3402
2012-08-29 11:04:19 -07:00
vrana
600e052e72 Dispatch event after building diff message
Summary:
We log time of running `arc diff`. It's easy with `--verbatim` or with users just confirming the message built in commit template.

With `--background`, I want to log only waiting time, not message editing time. This event should allow it.

Test Plan:
  $ arc diff

Haven't created the listener yet.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3394
2012-08-27 18:34:31 -07:00
epriestley
d83768a949 Minor, fix obvious typo in SVN patch workflow (@mroch). 2012-08-23 17:52:47 -07:00
vrana
7ba210f89a Create v2 libraries
Summary:
This is how it looks like now:

>     What do you want to name this library? x
> Writing '__phutil_library_init__.php' to 'x/__phutil_library_init__.php'...
> Usage Exception: This library is using libphutil v1, which is no longer supported. Run 'arc liberate --upgrade' to upgrade to v2.

Test Plan:
  $ arc liberate # in new dir

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3368
2012-08-22 15:43:54 -07:00
vrana
129339019f Always use lint advices
Summary:
We want to have a class of lint problems which are displayed to author and attached to revision but don't require excuse in diff workflow.
Lint advices already serve this purpose but no linters emit them because they need `--advice` flag to be processed.

By always enabling advices, we can switch more linters from warnings to advices and don't stop the diff workflow for them.

This diff also bumps down default severity of TODO rule.

Test Plan: Made lint advice mistake, ran `arc lint`.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, ide, Korvin

Differential Revision: https://secure.phabricator.com/D3364
2012-08-22 11:49:59 -07:00
epriestley
29ba92c0c2 Improve management of zlib dependency
Summary:
  - Add zlib functions to extension functions.
  - Provide a better error if the extension is actually missing.

Test Plan: Eyeballed it.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D3321
2012-08-20 17:26:42 -07:00
vrana
1779abef6f Link docs from message
Test Plan:
  phutil_console_confirm("See http://www.phabricator.com/docs/phabricator/article/Differential_User_Guide_Large_Changes.html for information");

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3317
2012-08-16 14:29:37 -07:00
vrana
13b64da47a Use console in unit inside diff 2012-08-15 15:54:34 -07:00
vrana
30bc4ca6e9 Don't ask for confirmation with unsound unit tests
Test Plan:
  $ arc diff # with unsound tests
  $ arc diff --ignore-unsound-tests # with unsound tests

Reviewers: epriestley, aran

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1515

Differential Revision: https://secure.phabricator.com/D2985
2012-08-15 15:49:50 -07:00
vrana
014521362f Fix lint warning 2012-08-15 14:27:04 -07:00
vrana
cae7631dff Warn about strstr() misuse
Summary: See D3296#1.

Test Plan:
New test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3297
2012-08-15 13:44:41 -07:00
vrana
34efe49e12 Warn before using strpos($a, $b) === 0
Summary:
I'm not sure if we are interested in this kind of linters ("Don't use slow function if there is a fast alternative").
I didn't find a case where `strpos() === 0` could be useful except [[ http://www.php.net/manual/en/mbstring.overload.php | mbstring.func_overload ]] craziness.

Test Plan:
New unit test.
Linted Arcanist, libphutil, Phabricator repositories, found no false positive and one real positive.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3296
2012-08-15 13:20:25 -07:00
Wez Furlong
390bb280e1 Add extraData field to test results
Summary:
See discussion on T1630.
extraData provides more scope for extensions to piggy-back
more data on the test results and have that pulled up to the UI.

We're using keys like "facebook:complexity" to store additional
data as part of the test results.

Test Plan:
Nothing in the codebase touches extraData at the moment, so
you'll just have to have faith/prove by inspection.

Reviewers: vrana, nh, tuomaspelkonen, epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T1630

Differential Revision: https://secure.phabricator.com/D3276
2012-08-15 10:34:29 -07:00
epriestley
a0d4430271 Detect use of f()[0] in lint
Summary: XHPAST doesn't currently parse most PHP 5.4 stuff, but it does parse this. Warn about it.

Test Plan:
Unit tests, and:

   Error  (XHP35) Use Of PHP 5.4 Features
    The f()[...] syntax was not introduced until PHP 5.4, but this codebase
    targets an earlier version of PHP. You can rewrite this expression using
    idx().

             365   public function lintPHP54Features($root) {
             366
             367     if (false) {
    >>>      368       id()[0];
                            ^
             369     }
             370

Reviewers: alanh, vrana

Reviewed By: alanh

CC: aran

Differential Revision: https://secure.phabricator.com/D3291
2012-08-15 04:36:50 -07:00
vrana
ab602e3a52 Pass lint and unit results to diff created event
Summary:
I need to run some jobs only if the tests hasn't been skipped.
I know that this could end up by passing more and more data to the event but this is all I need so far.

Test Plan: Dumped `unitResult` from the listener.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3259
2012-08-13 14:57:18 -07:00
epriestley
061a9f8cbd Implement base85 in a 32-bit safe way, without bcmath
Summary: See T1635 and the giant inline comment.

Test Plan: Ran unit tests on 32-bit and 64-bit machines.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1635

Differential Revision: https://secure.phabricator.com/D3250
2012-08-12 19:21:33 -07:00
epriestley
08b29ad23f Add test coverage to the base85 implementation
Summary: See T1635. I'm going to make an effort to rewrite this in a way that's safe in 32-bit PHP, i.e. without bcmath. Add test coverage to limit the chance I screw it up.

Test Plan: Ran unit tests.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1635

Differential Revision: https://secure.phabricator.com/D3249
2012-08-12 19:20:46 -07:00
epriestley
be3ce781bb 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
2012-08-12 08:50:01 -07:00
epriestley
c0c78c2ff4 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
2012-08-10 15:29:27 -07:00
epriestley
6b0af26b94 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
2012-08-10 15:16:02 -07:00
epriestley
a31d88ee82 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
2012-08-10 14:58:19 -07:00
vrana
6288bd6bcf 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
2012-08-10 14:46:08 -07:00
epriestley
edcc5cf6e1 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
2012-08-10 12:10:09 -07:00
epriestley
8f127af47c 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
2012-08-10 11:37:30 -07:00
epriestley
d2e0dc3e68 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
2012-08-09 18:39:41 -07:00
vrana
239b90a076 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
2012-08-09 17:44:57 -07:00
vrana
45fc4c992c 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
2012-08-08 19:04:59 -07:00
epriestley
678efa44c6 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
2012-08-08 12:58:27 -07:00
vrana
1ea9b8b02c 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
2012-08-08 12:19:14 -07:00
vrana
7e49cb4bfc 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
2012-08-08 12:07:12 -07:00
vrana
df81d25f1e 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
2012-08-08 12:04:15 -07:00
epriestley
cb5cfb28cd 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
2012-08-07 18:21:32 -07:00
epriestley
fae2adba9e 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
2012-08-06 11:56:01 -07:00
vrana
5d6e2a4e08 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
2012-08-05 18:39:32 -07:00
Alan Huang
877e6f743b 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
2012-08-03 12:00:54 -07:00
epriestley
4593af23a7 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
2012-08-01 11:02:20 -07:00
Evan Priestley
fca6f7549f Merge pull request #47 from aurelijus/master
JSHint linter issue on windows
2012-07-30 04:54:21 -07:00
Aurelijus
7f4ad7117a JSHint linter issue on windows
Summary: On windows there is no 'which', only 'where'

Test Plan: Run JSHint linter on windows and unix-like system.

Reviewers: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3096
2012-07-30 09:26:37 +02:00
Bob Trahan
333bb09e04 Make arc patch show the superior error message if you are not authenticated
Summary: ...by making requireAuthentication return true if conduit is required.

Test Plan: mv ~/.arcrc ~/.arcrcbak; arc patch D12121; <verify error message>

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T1522

Differential Revision: https://secure.phabricator.com/D3014
2012-07-29 11:57:36 -07:00
epriestley
5be656aa57 Include extension classes in the builtin list
Summary: Just like we ship with a list of extension functions, add a list of extension classes so people stop getting lint errors about DOMDocument just because some linter uses it.

Test Plan: Ran "arc lint".

Reviewers: vrana, btrahan, codeblock

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3082
2012-07-26 14:46:53 -07:00
Grigory
414e1e3e40 Added support for 'hg export' diff format
Summary: This diff format is used by de-facto mercurial GUI called "TortoiseHG".
It is available as the only way to copy diff into clipboard (right click commit,
select "export", select "copy patch". I added this format support into arcanist
so revisions in Differential can be created from TortoiseHG via simple copy-
paste. Unit test added, manually tested.

See: https://github.com/facebook/arcanist/pull/46

Reviewed by: epriestley
2012-07-26 10:08:49 -07:00
epriestley
8d9e1d8479 Minor, improve behavior of arc get-config. 2012-07-25 20:14:38 -07:00
Nick Harper
8e00d3cfaf Allow hyphens in arc library names
Summary:
We've had these in our library names and they're quite useful as word
separators. Allowing them shouldn't cause any trouble.

Test Plan:
ran arc liberate in an empty directory, and it didn't complain when I gave
it a name with a hyphen.

Reviewers: epriestley, vrana, jungejason

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3070
2012-07-25 18:37:25 -07:00