1. 09 Oct, 2018 2 commits
  2. 15 Sep, 2018 1 commit
  3. 14 Sep, 2018 1 commit
    • epriestley's avatar
      Make the Arcanist comment remover less aggressive about stripping instructional comments · 2650e862
      epriestley authored
      Summary:
      Ref T13098. See PHI858. If you write this at the end of a message in `arc diff`:
      
      ```
        Subscribers:
        #projectname
      
        # NEW DIFFERENTIAL REVISION
        # Describe the changes in this new revision.
        # ...
      ```
      
      ...we'll currently eat the `#projectname` as an instructional comment, even if it is followed by an empty line.
      
      Instead, stop eating stuff once we hit the first empty line. (We escape empty lines in comments already.)
      
      After T13098 I'll maybe adjust this to use a more explicit instruction escape, like `##`, since there's no reason we're bound to `#`.
      
      Test Plan: Added a unit test and made it pass.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13098
      
      Differential Revision: https://secure.phabricator.com/D19639
      2650e862
  4. 08 Sep, 2018 1 commit
  5. 06 Sep, 2018 1 commit
    • Joshua Spence's avatar
      Allow `willLintPaths` and `didLintPaths` to be overridden · 30b7835c
      Joshua Spence authored
      Summary: I'm not sure if the upstream will be interested in this change, but we are writing a linter which works by running an external command on the entire repository. This can't be done with `ArcanistExternalLinter` at the moment, which meant that we ended up copy-pasting most of `ArcanistFutureLinter`. This would be a lot easier if we could override `willLintPaths` and `didLintPaths`, but these methods are currently marked as `final`. An alternative solution would be some sort of `ArcanistLinter::transformPath` method.
      
      Test Plan: N/A
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: faulconbridge, Korvin
      
      Differential Revision: https://secure.phabricator.com/D19630
      30b7835c
  6. 25 Aug, 2018 1 commit
  7. 24 Aug, 2018 1 commit
    • epriestley's avatar
      Make the ArcanistBundle algorithm do what "diff -u" does when hunks are arguably mergeable · e1e93271
      epriestley authored
      Summary:
      Ref T13187. See PHI838. If two hunks are separated by 7 lines of context, we can render them as either:
      
      ```lang=diff
      + Hunk A
        Context 1
        Context 2
        Context 3
        Context 4
        Context 5
        Context 6
        Context 7
      + Hunk B
      ```
      
      ...or:
      
      ```lang=diff
      + Hunk A
        Context 1
        Context 2
        Context 3
      @@ +1,2 -3,4 @@
        Context 5
        Context 6
        Context 7
      + Hunk B
      ```
      
      Since we get the same number of output lines either way and the first one is more human-readable, we picked that one.
      
      However, `diff -u` does the second one. Since human-readability is probably less important than compatibility, change the behavior to be more similar to `diff -u`.
      
      Test Plan: Added unit tests for the edge cases with default parameters (6 context lines, 7 context lines) and made them pass.
      
      Reviewers: amckinley
      
      Maniphest Tasks: T13187
      
      Differential Revision: https://secure.phabricator.com/D19603
      e1e93271
  8. 06 Aug, 2018 1 commit
  9. 03 Aug, 2018 1 commit
  10. 20 Jul, 2018 1 commit
  11. 09 Jul, 2018 1 commit
  12. 23 Jun, 2018 1 commit
  13. 19 Jun, 2018 1 commit
  14. 08 Jun, 2018 1 commit
  15. 07 Jun, 2018 1 commit
    • epriestley's avatar
      In "arc patch", update submodules slightly later · df7313bd
      epriestley authored
      Summary:
      Ref T13151. See PHI648. With `arc patch --nobranch`, we update submodules a little too early.
      
      I //believe// it is safe to just update them a little later, after the intermediate branch management logic runs.
      
      Test Plan: Ran `arc patch --nobranch`, saw submodule update run later. Not 100% sure this doesn't cause weird issues, but I can't anticipate any.
      
      Reviewers: amckinley, jmeador
      
      Reviewed By: jmeador
      
      Maniphest Tasks: T13151
      
      Differential Revision: https://secure.phabricator.com/D19475
      df7313bd
  16. 05 Jun, 2018 1 commit
    • Tim McJilton's avatar
      [ARCUNIT] Set the ConfigurationManager of ConfigurationDrivenUnitTestEngines · b199ca80
      Tim McJilton authored
      Summary:
      The Configuration Manager is supported by ArcanistUnitTestEngine but not support by the ArcanistConfigurationDrivenUnitTestEngine.
      
      Added the configuration manager as one of the initially set properties of an ArcUnitTestEngine created by the ArcanistConfigurationDrivenTestEngine
      
      Test Plan: Ran arc unit against a project without the change, verified the Configuration was none. Added this change and ran again and verified it was set
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D19465
      b199ca80
  17. 14 May, 2018 2 commits
    • epriestley's avatar
      (stable) Promote 2018 Week 19 · 733ac805
      epriestley authored
      733ac805
    • epriestley's avatar
      Allow diff generation via ArcanistBundle to be limited to an approximate maximum byte size · d581c453
      epriestley authored
      Summary:
      Ref T13137. See PHI592. When you have a diff with 600MB of videos, we want to bail out of diff generation early (as soon as we realize what we're dealing with), not build an 800MB text diff in memory and then throw it away.
      
      Support bailout //during// diff generation once we realize we're in over our heads.
      
      This is approximate, but since the limit is fairly large (512KB by default) it isn't too important to be precise.
      
      Test Plan: Rigged some callers to set various byte limits, generated diffs including diffs with large binaries. Got an appropriate diff or exeception depending on how low the limit was.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13137
      
      Differential Revision: https://secure.phabricator.com/D19444
      d581c453
  18. 09 May, 2018 1 commit
    • epriestley's avatar
      Raise the intraline diff hard limit from 80 to 100 characters · a1aec701
      epriestley authored
      Summary: Fixes T1246. See PHI637. See T13137. Computers have gotten a bit faster so we can probably bump this up a little and see if it causes problems. This is `O(N^2)` so the this should be less than twice as expensive in the worst case.
      
      Test Plan:
      Created a diff affecting characters on a very long line separated by more than 80 but fewer than 100 characters, got a good intraline diff out of it:
      
      {F5605162}
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T1246
      
      Differential Revision: https://secure.phabricator.com/D19442
      a1aec701
  19. 27 Apr, 2018 3 commits
    • epriestley's avatar
      (stable) Promote 2018 Week 17 · 8794ce1e
      epriestley authored
      8794ce1e
    • epriestley's avatar
      Slightly improve base85 performance for 64-bit systems · a6045481
      epriestley authored
      Summary:
      Ref T13130. I wasn't able make this much better, but it looks like this is about ~20% faster on my system.
      
      This kind of thing is somewhat difficult to micro-optimize because XHProf tends to over-estimate the cost of function calls. In XHProf, this looks much much faster than the old version (~100% faster) but the actual cost of `bin/conduit call --method differential.getrawdiff` hasn't improved that much. Still, it seems consistently faster across multiple runs.
      
      Test Plan:
        - Pulled binary diffs over Conduit with `bin/conduit call --method differential.getrawdiff`.
        - Verified that they are byte-for-byte identical with the pre-change diffs, and look like they're ~20% faster.
        - Profiled the differences and saw a far more dramatic improvement, but I believe XHProf is exaggerating the effect of this change because it tends to overestimate function call cost.
        - Ran unit tests (from D19407), got byte-for-byte identical output under both 32bit and 64bit mode.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13130
      
      Differential Revision: https://secure.phabricator.com/D19408
      a6045481
    • epriestley's avatar
      Restructure base85 unit tests to support inlining and multiple encoding pathways · bcab677a
      epriestley authored
      Summary:
      Ref T13130. I want to take a crack at improving performance here, but two possible approaches (inlining the actual encoding; using integers if they're big enough) aren't easy to test right now.
      
      Restructure the tests so they can support these kinds of refactoring.
      
      The "32bit" and "64bit" modes currently do the same thing, but I expect to introduce introduce separate encoding pathways in a future change, if the profiler says it actually helps.
      
      (I'll hold this and everything that comes after it until I make meaningful performance improvements.)
      
      Test Plan: Ran `arc unit`, got passes on tests.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13130
      
      Differential Revision: https://secure.phabricator.com/D19407
      bcab677a
  20. 20 Apr, 2018 2 commits
    • epriestley's avatar
      (stable) Promote 2018 Week 16 · 23f199bf
      epriestley authored
      23f199bf
    • Alex Vandiver's avatar
      Correctly parse `git status --porcelain=2` output with filenames with spaces · ad3087e5
      Alex Vandiver authored
      Summary:
      Filenames are last in `git status --porcelain=2` lines; they
      are not escaped in any way, despite the fields being
      whitespace-delimited.  `explode` thus happily chops apart filenames
      with spaces in them, causing later git operations to operate only on
      the filename up to the first space.
      
      Split the lines into the right number of elements -- in all cases,
      this is one more than the index we're using, since filenames come last.
      
      Test Plan:
      Altering a file with a space in its path, and running `arc diff -a`.
      
      Added tests.
      
      Reviewers: #blessed_reviewers, epriestley
      
      Reviewed By: #blessed_reviewers, epriestley
      
      Subscribers: Korvin, epriestley
      
      Differential Revision: https://secure.phabricator.com/D19389
      ad3087e5
  21. 08 Apr, 2018 1 commit
  22. 05 Apr, 2018 1 commit
  23. 03 Apr, 2018 1 commit
    • epriestley's avatar
      Improve argument parsing for "arc patch --revision Dxxx" · e44a2d3a
      epriestley authored
      Summary: See PHI527. Ref T13116. The `--revision` flag currently fails if the argument is in the form `D123` instead of `123`. Normalize monogram arguments.
      
      Test Plan: Ran `arc patch --revision Dxxx`, `arc patch --revision xxx`, `arc patch --revision xxx --diff yyy`, `arc patch`; got good behavior on the good ones and sensible error messages on the other ones.
      
      Maniphest Tasks: T13116
      
      Differential Revision: https://secure.phabricator.com/D19292
      e44a2d3a
  24. 31 Mar, 2018 1 commit
  25. 26 Mar, 2018 2 commits
  26. 09 Mar, 2018 1 commit
  27. 03 Mar, 2018 1 commit
    • Nathan LeClaire's avatar
      Add support for Go 1.10 (cached) output · dcd7ef66
      Nathan LeClaire authored
      Summary:
      In Go 1.10 the output for tests was changed to have also a "(cached)" mode in
      addition to the normal timing info printed. This is on by default. This adds
      support for parsing these lines instead of erroring out on the regex.
      
      Test Plan: Have a unit test included, and will continue to poke at it locally.
      
      Reviewers: epriestley, #blessed_reviewers
      
      Reviewed By: epriestley, #blessed_reviewers
      
      Subscribers: Korvin
      
      Differential Revision: https://secure.phabricator.com/D19161
      dcd7ef66
  28. 16 Feb, 2018 1 commit
  29. 13 Feb, 2018 1 commit
  30. 10 Feb, 2018 1 commit
  31. 05 Feb, 2018 1 commit
    • epriestley's avatar
      Clarify what "--everything" means in "arc lint" and "arc unit" · 34910942
      epriestley authored
      Summary:
      Fixes T13061. Both `arc lint` and `arc unit` accept an `--everything` flag, but the documentation isn't quite clear about what these flags do.
      
      They act as though every //tracked// file in the repository (`git ls-files`, `hg manifest`, or `svn list -R`) is included in the argument list.
      
      They do not lint/test ignored files (and I think almost all users would be very surprised if they did).
      
      They also don't lint/test untracked files (files you have not yet used `git add`, `svn add`, or `hg add` on). This is slightly more contentious but we have good reasons for doing it (e.g., `git ls-files` often outperforms `find .` by a large margin) and I believe users very rarely use `--everything` in a situation where they have untracked files. The only real exception I can come up with is linter configuration/development, as in PHI343, and it seems okay to have a slightly surprising behvaior here.
      
      Make the documentation more clear about what is in scope.
      
      We could also rename these to `--nearly-everything` or whatever, but I think the name is probably clear enough given current information about how confusing this is (specifically: only rarely, in unusual cases).
      
      Test Plan:
        - Grepped for documentation about these flags.
        - Ran `arc help lint`, `arc help unit`, `arc unit --everything x`, `arc lint --everything x` and read all the new messages.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T13061
      
      Differential Revision: https://secure.phabricator.com/D18989
      34910942
  32. 19 Jan, 2018 1 commit
  33. 16 Jan, 2018 1 commit
    • epriestley's avatar
      Add trailing tabs when generating synthetic Git diffs for files with spaces · 2e023322
      epriestley authored
      Summary:
      Fixes T8768. See PHI294. See that task for more details.
      
      Git, Mercurial, `diff`, and `patch` have conspired to make things weird. To correctly handle files with spaces in the way everything else does and expects, we need to emit semantic trailing whitespace literals.
      
      Test Plan:
      - Created a file with spaces in it in a Mercurial repositroy, committed it, diffed it into a revision.
      - Used `arc patch` to apply the change to a clean copy of the repository.
      - Before patch: Mercurial incorrectly creates a file named `X`, not a file named `X Y.txt`.
      - After patch: `arc patch` commit is identical to genuine commit.
      - Also added test coverage. The other general behaviors here are fairly well covered already.
      
      Reviewers: amckinley
      
      Reviewed By: amckinley
      
      Maniphest Tasks: T8768
      
      Differential Revision: https://secure.phabricator.com/D18869
      2e023322
  34. 26 Dec, 2017 1 commit