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

2022 commits

Author SHA1 Message Date
epriestley
2831d075c0 Remove only trailing "#" and empty lines as comments from CLI editors
Summary: Don't strip numbered lists in comment bodies, etc.

Test Plan: Unit tests, meta-testing this diff.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1124

Differential Revision: https://secure.phabricator.com/D2262
2012-04-18 06:08:41 -07:00
epriestley
da4b6b5799 Rename "arc mark-committed" to "arc close-revision"
Summary:
  - Replace SVN-specific language with VCS-agnostic language.
  - Add new "arc close-revision", works exactly like "arc mark-committed" but with agnostic language.
  - Use status constants, not status strings.
  - Mark "arc mark-committed" deprecated.
  - Remove deprecated "arc merge".

Test Plan: Ran "arc mark-committed", "arc close-revision".

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, Makinde

Maniphest Tasks: T909

Differential Revision: https://secure.phabricator.com/D2244
2012-04-17 13:51:10 -07:00
vrana
c320e0987d Don't advice using --less-context if already used
Test Plan:
  arc diff
  arc diff --less-context

Reviewers: blair

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2233
2012-04-14 21:50:52 -07:00
epriestley
ea0f737e85 Drop <project + branch> herusitic from Git
Summary: See rage in T1117. Don't use the <project + branch> heuristic anymore..

Test Plan: Ran "arc diff --strict HEAD^" on a commit stacked on top of this one, got no matches.

Reviewers: btrahan, vrana, simpkins, beng

Reviewed By: btrahan

CC: aran, avive

Maniphest Tasks: T1117

Differential Revision: https://secure.phabricator.com/D2221
2012-04-13 10:55:28 -07:00
epriestley
56cdc31426 Fix some --only / --preview / SVN issues with Arcanist
Summary:
  - Historically, "--preview" was forbidden under SVN. No reason for that now.
  - The "--auto" patch moved the "--preview" / "--only" checks later than they should be.
  - Fix an issue with Conduit query construction in SVN.

Test Plan: Ran "arc diff --preview" in an SVN working copy. Ran "arc diff" in an SVN working copy.

Reviewers: svemir, btrahan, vrana, jungejason

Reviewed By: svemir

CC: aran

Differential Revision: https://secure.phabricator.com/D2218
2012-04-12 10:32:54 -07:00
Git user
83ad377bdb Read default relative commit from scratch file with newline
Summary:
When people create the .arc/default-relative-commit scratchfile with $EDITOR of
choice, their editor usually puts a newline at the end, which breaks arc diff.
We should trim the newline before using the contents of the scratchfile.

Test Plan:
ran arc diff in a working copy that contained a .arc/default-relative-commit
with a newline

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2209
2012-04-11 17:44:57 -07:00
epriestley
d2915d8c5f Don't let "arc which" identify the working copy as belonging to a different project
Summary: If you have two projects (say, libphutil and arcanist) and you prepare a patch for one of them on branch "master", run "arc diff", and then prepare a patch for the other one on the same branch, "arc diff" will try to update the first revision when you run it. Instead, make it smart enough to stay within arc projects.

Test Plan: Ran "arc which" in circumstances where it previously generated a false positive, no false positive.

Reviewers: btrahan, vrana, jungejason

Reviewed By: jungejason

CC: aran

Maniphest Tasks: T1100

Differential Revision: https://secure.phabricator.com/D2199
2012-04-10 16:06:57 -07:00
epriestley
7ea51b6bb7 If a Git upstream is configured for the current branch, always use that as the default relative commit
Summary: See discussion in D1861.

Test Plan: Ran "arc diff" on master, got an upstream-based relative commit. Ran "arc diff" on a feature branch, got a config-based relative commit. Ran "arc diff x", got an argument-based relative commit.

Reviewers: btrahan, vrana, davidreuss, elgenie

Reviewed By: davidreuss

CC: aran

Differential Revision: https://secure.phabricator.com/D2192
2012-04-10 15:33:31 -07:00
Ben Gertzfield
2c02e79df4 Allow defining aliases in .arcconfig.
Summary:
For Objective-C repositories, we want to provide aliases to
arc diff --amend-autofixes by default.

This adds the ability to define aliases in .arcconfig (overridden
by any specified in the user config, of course).

Test Plan:
Tested defining alias with nothing in .arcconfig, with
an alias in .arcconfig.  Tested arc alias outside of working
repository.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2191
2012-04-10 12:42:13 -07:00
Ben Gertzfield
14d49d2565 Add ArcanistLintSeverity::SEVERITY_AUTOFIX.
Summary:
Xcode (a popular code editor on Mac OS X) has no facility
to trim trailing whitespace automatically.

This adds a new lint severity "AUTOFIX" that's between
WARNING and ERROR. When running the linter, any lint message
whose severity is AUTOFIX will automatically be patched.

Furthermore, if all lint messages returned from the engine are
AUTOFIX, we'll automatically amend HEAD with the patch.

Test Plan:
arc lint on files with and without trailing whitespace,
with and without UTF-8 contents to confirm those still error

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2125
2012-04-10 12:42:09 -07:00
epriestley
55dce2beeb Make --auto creates actually work in SVN and HG
Summary:
--auto doesn't work right now on the implicit --create pathway in SVN and HG because we hit these conditions.

Also improve a message.

Test Plan: Ran "arc diff" in unaffiliated working copies in HG and SVN.

Reviewers: svemir, btrahan, vrana, jungejason

Reviewed By: svemir

CC: aran

Differential Revision: https://secure.phabricator.com/D2187
2012-04-10 12:06:41 -07:00
vrana
1c81cd7615 Fix docs links after D2181 and D2182
Test Plan:
  diviner .

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2189
2012-04-10 11:32:09 -07:00
epriestley
5068e8ad56 Provide a "loadFileDataCallback" for ArcanistBundle
Summary: See next diff for an explanation of this issue.

Test Plan: See next diff.

Reviewers: Makinde, btrahan, vrana, jungejason

Reviewed By: Makinde

CC: aran

Differential Revision: https://secure.phabricator.com/D2174
2012-04-09 17:34:45 -07:00
vrana
f3fc75fcb9 Unify links to www.phabricator.com and phabricator.com
Test Plan: Visit http://www.phabricator.com/docs/phabricator/article/Arcanist_User_Guide:_Configuring_a_New_Project.html.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1096

Differential Revision: https://secure.phabricator.com/D2171
2012-04-09 14:27:51 -07:00
Evan Priestley
94c695cf0f Merge pull request #25 from Koolvin/arc_close
Add arc close command
2012-04-09 08:46:14 -07:00
vrana
a2c125d4ef Call phutil_passthru() instead of passthru() in liberate
Summary: Solves T1085 and is probably slightly better anyway.

Test Plan:
  arc liberate

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T1085

Differential Revision: https://secure.phabricator.com/D2151
2012-04-08 21:34:36 -07:00
Koolvin
e7d9ff18d3 Add arc close command
Summary:
```arc close T1088 --status wontfix --message "I'm not going to fix this."```
T1088 is the test task to screw with, so feel free.

Test Plan: ```arc close T1088 --status [ resolved | wontfix | invalid | duplicate | spite | open ] -m "Message"```

Reviewers: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2162
2012-04-08 20:57:38 -07:00
epriestley
2c599f8928 Raise lint warning on missing space after comma
Summary: Saw this in a diff somewhere; complain about it.

Test Plan: Unit coverage.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1060

Differential Revision: https://secure.phabricator.com/D2153
2012-04-08 16:09:11 -07:00
Evan Priestley
f346ab7f9a Merge pull request #24 from Koolvin/arcpatch-D1943
Add arc tasks command
2012-04-07 17:48:32 -07:00
Koolvin
b553af2763 Add arc tasks command
Summary:
Added `arc tasks`:

%%%arc tasks
arc tasks --view-all    // View Open and Closed Tasks
arc tasks --by-status   // Group By Status
arc tasks --by-priority // Group By Priority%%%

Test Plan: Connect to conduit and run arc tasks >> make sure you have tasks =p

Reviewers: epriestley, indiefan

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T749

Differential Revision: https://secure.phabricator.com/D1943
2012-04-07 17:40:25 -07:00
epriestley
038d8e86d6 Make "--auto" the default for "arc diff"
Summary:
NOTE: This is a disruptive change to how 'arc diff' behaves by default.

Many years ago, Differential (then DiffCamp) supported SVN. Someone added a "-i" mode to the "diffcamp" script so you could "git show | diffcamp -i", and thus we were
forever bound to storing metadata in commit messages.

But this isn't a common use case outside of Facebook + git-svn, and isn't very flexible. We now have a great deal of flexibility to identify revisions based on
hashes, branch names, etc, and to sync metdata from web to CLI and back. I want to jettison the commit-message-bound artifacts of the tool's history, and move to a
more flexible, modern workflow.

I added "--auto" a while ago, which figures out if you want to create or update a diff automatically, and then prompts you for whatever data it needs, reading
information if it can from commit messages in the range. This is a vastly better workflow in general, especially for SVN and Mercurial users (who currently need to
jump to the web UI to create revisions). It's better for git users too, since they don't need to use template commits and don't have to muck with or configure
templates. However, it's a nontrivial change to git users' core workflow that is clearly different in more ways than it is clearly better.

  - This might be contentious, but probably not toooo much, I hope?
  - I also deleted the (fairly ridiculous) workflow where we sync commit message changes from the working copy. I think two or three people will be REALLY upset about
this but I have no sympathy. "--edit" covers this and has been around for like two years now, and making the commit message and web dual-authoritative was always a bad
idea that we only ever half-accommodated anyway (see giant swaths of removed TOOD nonsense).

Test Plan:
  - I've been using "--auto" exclusively for like a month, it seems to work well.
  - Created/updated SVN, Git and HG diffs with "arc diff" under this workflow.

Reviewers: btrahan, jungejason, nh

Reviewed By: btrahan

CC: Makinde, vrana, zeeg, mbautin, aran, yairlivne, 20after4

Maniphest Tasks: T614

Differential Revision: https://secure.phabricator.com/D2080
2012-04-07 10:39:27 -07:00
epriestley
39c3a0c43a Fix an issue with "arc diff --update" in Git on immutable workflows
Summary:
This should be a raw text block, not a parsed message object.

I broke this in rARC1f13e022cdc9bf4859274a83784bd615caf62ef9 when I improved Mercurial support.

See: https://github.com/facebook/arcanist/issues/23

Test Plan: (Will update this diff...)

Reviewers: vrana, jungejason, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2136
2012-04-07 10:36:16 -07:00
vrana
8295bddba7 Bump required PHP version
Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2127
2012-04-06 15:26:32 -07:00
vrana
a5f0323d5c Return $this from didApplyPatch() shortcut
Test Plan: https://secure.phabricator.com/diffusion/ARC/browse/master/src/lint/patcher/ArcanistLintPatcher.php;bd7dc8abaa7bb4c0$75?view=blame

Reviewers: epriestley, jungejason

Reviewed By: jungejason

CC: aran

Differential Revision: https://secure.phabricator.com/D2101
2012-04-04 15:11:52 -07:00
epriestley
bd7dc8abaa Allow configuration of a weak per-project default relative commit
Summary: Allows you to set a default in .arcconfig. This default is overriden by any .arc/ setting.

Test Plan: Ran "arc diff" with a .arcconfig setting but no .arc/ setting, got a diff against the specified relative commit.

Reviewers: nh, btrahan

Reviewed By: nh

CC: aran

Differential Revision: https://secure.phabricator.com/D2093
2012-04-03 16:06:43 -07:00
vrana
8971a91444 Return $this from setters
Summary:
Most setters returns `$this` but some don't.
I guess it's not by purpose.

Test Plan:
  arc lint

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2084
2012-04-02 18:47:59 -07:00
epriestley
39b3778287 Revenge of the git relative commit default
Summary:
The default of "arc diff" to "arc diff HEAD^" in git is universally confusing to everyone not at Facebook.

Drive the default with configuration instead. Even at Facebook, "origin/trunk" (or whatever) is probably a better default than "HEAD^".

See D863 for the last attempt at this.

NOTE: This is contentious!

Test Plan: Ran "arc diff", got prompted to set a default. Ran "arc diff" from a zero-commit repo, got sensible behavior

Reviewers: btrahan, vrana, nh, jungejason, tuomaspelkonen

Reviewed By: btrahan

CC: aran, epriestley, zeeg, davidreuss

Maniphest Tasks: T651

Differential Revision: https://secure.phabricator.com/D1861
2012-04-02 16:52:20 -07:00
epriestley
2a66d6bde9 Minor, correct arc handling of relative symlinking (from @makinde). 2012-04-02 16:33:36 -07:00
Nick Harper
0d76ac6968 [arcanist] properly handle arguments with spaces
Summary:
The new wrapper shell script that is bin/arc does not correctly
handle arguments with spaces in them.

Test Plan:
added a print_r($argv) to scripts/arcanist.php and ran bin/arc -m
"testing something" to see that "testing something" got passed in as
one argument instead of two.

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran

Differential Revision: https://secure.phabricator.com/D2066
2012-03-30 20:34:49 -07:00
vrana
a0c5835a43 Whitespace 2012-03-30 14:55:55 -07:00
epriestley
3ae1bf1a8c Add a lint check for clobbering locals with iterators
Summary:
See D2049, D2050. Identify reuses of locals as iterator variables. Before raising an error, we require:

  - Variable is declared before the loop.
  - Variable is used after the loop, ignoring uses as an iterator variable.

I think this identifies all problems with a very low false positive rate (the false positives are suspicious/unconventional code, but not necessarily errors).

Also fix an issue identified by the linter.

Test Plan:
  - Verified this identified the bugs in D2049 and D2050.
  - Ran linter against libphutil/, arcanist/ and phabricator/ (see D2051, this, and next diff).
  - Ran unit tests.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley, jungejason

Differential Revision: https://secure.phabricator.com/D2052
2012-03-29 13:21:18 -07:00
epriestley
ff94d699fe Minor Arcanist fixes
Summary:
Addresses concerns in rARCefb8219196abf047f14b505959e54d078e1df6d3:

  - As I recall, the intent of "generateFile" was that these warnings would replace all the other warnings for that file, under the theory that if one warning caused regeneration of
the file the other warnings were irrelevant.
  - However, this code never had any effect and I haven't seen any issues with the behavior.
  - So, just remove it.

Addresses concerns in rARC070e963d1c26879e47eab19a2377e388c2f166c5:

  - Ran "arc liberate --all".

Test Plan: Ran "arc lint" after removing an "__init__.php" with and without a "fixed" generateFile block, there was no behavioral change. So I just nuked it.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2049
2012-03-29 13:21:10 -07:00
vrana
30f036c6b9 Use assert_instances_of()
Summary: D2042

Test Plan:
- `arc lint` with dependent message
- `arc liberate`
- `arc diff`

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2048
2012-03-29 08:35:49 -07:00
vrana
edb6a38389 Avoid double indenting in console wrapping
Summary:
D2016 changed the behavior of `phutil_console_wrap()`.
The new behavior is better so I am fixing callsites instead of the function.

Test Plan:
  arc help help

Verify that the options descriptions is not indented with 28 spaces.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2047
2012-03-28 22:28:15 -07:00
epriestley
070e963d1c Minor, squelch a warning about reference variables.
Auditors: vrana
2012-03-26 10:03:59 -07:00
epriestley
9db7f294ef Show a better expected vs actual for tests with large output
Summary: When tests have a lot of output, show a diff of the expected/actual.

Test Plan: Used this when developing D2016 to examine large output usefully.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D2017
2012-03-26 09:43:47 -07:00
epriestley
b67ce7e534 Raise a better error message when "arc patch" fails because of filename case changes
Summary: Git can't apply filename case change patches on case-insensitive filesystems. Some day we could manually do this ourselves, but it's fairly rare and complicated -- just raise a useful warning.

Test Plan: Tried to apply a case-changing patch, got a good error.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T618

Differential Revision: https://secure.phabricator.com/D2015
2012-03-26 09:43:37 -07:00
epriestley
4ca58d1129 Improve compatibility of "arc patch" on Windows
Summary: Can't use (cd ...) on windows.

Test Plan: Ran "arc patch" successfully.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124, T1034

Differential Revision: https://secure.phabricator.com/D1987
2012-03-25 09:32:20 -07:00
epriestley
1acbe76cbf Minor, fix clowniness. 2012-03-23 08:56:38 -07:00
epriestley
3991e679e7 Minor, fix inverted condition. 2012-03-23 08:53:47 -07:00
epriestley
61957c4ade Update 'arc land': add --squash, add default config, fix windows compat
Summary:
  - We have "--merge", added similar "--squash" for completeness.
  - Switched away from (cd .. && ...) pattern which is not functional on Windows.
  - Allow a default --onto to be specified, if master isn't the best default for a project.

Test Plan:
  - Did --hold --keep dry runs, changes landed properly.
  - Set a default onto branch in .arcconfig.
  - Ran --merge and --squash together to verify conflict configuration.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, gschmidt

Maniphest Tasks: T124, T1033

Differential Revision: https://secure.phabricator.com/D2001
2012-03-23 08:45:28 -07:00
epriestley
e31bc81b6c Update 'arc land': add --squash, add default config, fix windows compat
Summary:
  - We have "--merge", added similar "--squash" for completeness.
  - Switched away from (cd .. && ...) pattern which is not functional on Windows.
  - Allow a default --onto to be specified, if master isn't the best default for a project.

Test Plan:
  - Did --hold --keep dry runs, changes landed properly.
  - Set a default onto branch in .arcconfig.
  - Ran --merge and --squash together to verify conflict configuration.

Reviewers: btrahan

CC: aran, epriestley

Maniphest Tasks: T124, T1033

Differential Revision: https://secure.phabricator.com/D2001
2012-03-22 18:28:35 -07:00
epriestley
3bff9417e9 Set relative commit early in "arc diff"
Summary:
In Mercurial, we figure out if the working copy is dirty with "hg diff", and do a somewhat expensive merge-base / outgoing operation if the relative commit isn't set. We can set the relative commit earlier and avoid some extra work.

We also may incorrectly cache this state (diff from merge-base of outgoing to tip) and pass the wrong rev and file dirty list to the linters.

Test Plan: Made commits which changed (A, B) and then (A). Ran "arc diff tip^". Before this change, observed full outgoing + merge base resolve and both "A" and "B" passed to lint. Observed execution of fewer commands and lint executing against "A" only after this change.

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1998
2012-03-22 17:22:52 -07:00
epriestley
87308eb34e Fix a doc link in arcanist
Summary:
Missed this when I moved the document a while ago. See <https://github.com/facebook/arcanist/issues/21>.

NOTE: The actual document isn't very helpful in figuring out the error. I'll address this in a followup.

Test Plan: Hit error, clicked link, got docs.

Reviewers: btrahan

Reviewed By: btrahan

CC: jmhsieh, aran, epriestley

Differential Revision: https://secure.phabricator.com/D1989
2012-03-22 12:25:37 -07:00
epriestley
7c3f8b3041 Resolve arc location through symlinks-of-symlinks, etc
Summary:
Stole this from stackoverflow; seems to work?

http://stackoverflow.com/questions/59895/can-a-bash-script-tell-what-directory-its-stored-in

Test Plan: Ran a symlink to bin/arc.

Reviewers: btrahan, zeeg

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1968
2012-03-21 18:04:35 -07:00
epriestley
e8accf4b9f Instead of looping forever, resolve the relative commit
Summary: Obvious error in refactoring code around getCanonicalRevisionName() in D1954.

Test Plan: Ran "arc diff" without looping. Can you verify this fixes your case?

Reviewers: Makinde, btrahan

Reviewed By: Makinde

CC: aran, epriestley

Maniphest Tasks: T1025

Differential Revision: https://secure.phabricator.com/D1970
2012-03-21 11:07:15 -07:00
epriestley
27725b073e Replace "bin/arc" with a shell script
Summary: Currently, we use a symlink. Under git bash, it tries to run the symlink. This is not a recipe for success. Instead, use a trivial wrapper script.

Test Plan: Ran "arc" from git bash.

Reviewers: Makinde, btrahan, Koolvin

Reviewed By: Koolvin

CC: aran, epriestley

Maniphest Tasks: T124

Differential Revision: https://secure.phabricator.com/D1939
2012-03-19 19:23:41 -07:00
epriestley
31a54d9b4a When a mercurial working copy has uncommitted changes, prompt to include them in "arc diff"
Summary:
See task. Allow mercurial users to diff with uncommitted changes.

  - By default, commit range is merge-base of `hg outgoing` to `.` (dirstate).
  - You can get JUST dirstate with `arc diff tip` or similar.
  - This ended up being a giant mess various other changes to deal with empty `hg outgoing` and empty dirstate.

Test Plan: Diffed with uncommitted changes, got sensible prompts and results.

Reviewers: Makinde, btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T998

Differential Revision: https://secure.phabricator.com/D1954
2012-03-19 19:17:10 -07:00
epriestley
b076b1c02a Use libphutil __init_script__.php to initialize Arcanist
Summary:
See D1950, same patch for arc/.

NOTE: I'm moving the ob_get_level() stuff to libphuil.

Test Plan: Ran "arc".

Reviewers: btrahan, killermonk

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1951
2012-03-19 19:17:03 -07:00
epriestley
cdcb5cc25d Minor, address @btrahan feedback. 2012-03-19 19:16:50 -07:00