From 6479e7f6c01117fb0d8e76abf26b48c06a560800 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 11 Jul 2018 16:56:08 -0700 Subject: [PATCH 01/28] Add draft of "Consolidate Core Atom Packages" RFC --- docs/rfcs/003-consolidate-core-packages.md | 332 +++++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 docs/rfcs/003-consolidate-core-packages.md diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md new file mode 100644 index 000000000..a96b8d947 --- /dev/null +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -0,0 +1,332 @@ +# Consolidate Core Atom Packages + +## Status + +Proposed + +## Summary + +Atom's official distribution is comprised of 91 core packages which provide its built-in functionality. These packages currently live in their own independent repositories in the Atom organization, all with their own separate issues, PRs, releases, and CI configurations. This RFC proposes that by consolidating most, if not all, of these core packages back into the `atom/atom` repo, we will see the following benefits: + +- Less confusion for new contributors +- Simpler core package contribution experience +- Greatly reduced burden for maintainers + +## Motivation + +Let's cover each of the bullet points mentioned above: + +### Less confusion for contributors + +Imagine that a new contributor wants to add a small new feature to the `tree-view` package. The first place they are likely to look is the `atom/atom` repository. Scanning through the folders will lead to a dead end, nothing that looks like `tree-view` code can be found. They might take one of the following steps next: + +- By reading README.md, maybe they will decide to click the link to the Atom Flight Manual and maybe_ find the [Contributing to Official Atom Packages](https://flight-manual.atom.io/hacking-atom/sections/contributing-to-official-atom-packages/) page there. +- They could read the CONTRIBUTING.md file which [has a section](https://github.com/atom/atom/blob/master/CONTRIBUTING.md#atom-and-packages) that explains where to find the repos for core packages and how to contribute, but we don't really have a clear pointer to that in our README.md +- If they don't happen to find that page, they might use Google to search for "atom tree view" and find the atom/tree-view repo and _maybe_ read the CONTRIBUTING.md file which sends them to Atom's overall contribution documentation +- They might go to the Atom Forum or Slack community to ask how to contribute to + +Having all of the core Atom packages represented in a top-level `packages` folder, even if they don't actually live in the repo, will go a long way to making the core package code more discoverable. + +### Simpler core package contribution experience + +Separating core Atom features out into separate repositories and delivered via `apm` is a great idea in theory because it validates the Atom package ecosystem and gives developers many examples of how to develop an Atom package. It also gives Atom developers real-world experience working with Atom's APIs so that we ensure community package authors have the same hackability that the Atom developers enjoy. + +On the other hand, having these packages live in separate repositories and released "independently" introduces a great deal of overhead when adding new features. Here is a comparison of the current package development workflow contrasted to what we could achieve with consolidated packages: + +#### Current Package Development Workflow + +For example, to add a single feature to the `tree-view` package, one must: + +1. Fork and clone the `tree-view` repository to their computer (making sure to pull the commit relevant to the version of Atom they are working with) +1. Run `apm install` and `apm link` inside of the repo folder +1. Make their desired changes to the code +1. Open a PR to the `tree-view` repo and wait for CI to pass and a maintainer to review it +1. Work with maintainers to get the PR approved and merged + +After this is finished, an Atom maintainer must take the following steps + +1. Clone the `tree-view` repo +2. Run `apm publish` to publish a new release of the package +3. Edit `package.json` in the Atom repo to reflect the new version of `tree-view` +4. Commit and push the changes to the relevant branch where the change belongs (`master` or `1.nn-releases`) + +If `tree-view` was moved into the `atom/atom` repository + +#### Simplified Package Development + +If we were to move `tree-view` (or any other core Atom package) back into `atom/atom`, the development workflow would look more like this: + +1. Fork and clone `atom/atom` and switch to a release branch if necessary +1. Build Atom and launch it in dev mode +1. Make desired changes to the code in `packages/tree-view` +1. Open a PR on `atom/atom` and wait for CI to pass and a maintainer to review it +1. Work with maintainers to get the PR approved and merged + +At this point, the change is merged into Atom and ready for inclusion in the next release. + +### Greatly reduced burden for maintainers + +Since packages all have their own repositories, this means that we have to watch 91 different repos for issues and pull requests. This also means that we have to redirect issues filed on `atom/atom` to the appropriate repository when a user doesn't know where it belongs. Even more importantly, there's not an easy way to prioritize and track issues across the Atom organization without using GitHub projects. + +Also, as mentioned above, there's the added duty of doing the package "version dance" when we merge any new PRs to a package repository: publish the package update, update `package.json` in Atom. It's very easy to forget to do this and not have community contributions included in the next Atom release! + +The more core packages live in `atom/atom`, the less work Atom maintainers have to do overall. + +## Explanation + +Many of Atom's core packages now live in the core `atom/atom` repository. To the Atom user, this change will be imperceptible as these packages still show up in the list of Core Packages in the Settings View. For maintainers and contributors, there will be less juggling of repositories and no more publishing of updates to these packages with `apm`. + +Contributors now clone and build `atom/atom` to work on improvements to core packages. They will no longer have to use `apm link` in dev mode to test changes they make to packages in the repo's `packages` folder. + +When a contributor sends a PR to `atom/atom` that only affects files in a folder under `packages`, only the specs for the relevant package folders will be executed using Atom's CI scripts. This means that a full Atom build will not be required when no Atom Core code is changed in a PR. Package specs are also now run against all 3 OSes on Atom `master` and release builds. + +Core packages that aren't consolidated still have folders under `packages` with README.md files that point to the home repository for that package. + +## Drawbacks + +One possible drawback of this approach is that there might be some initial confusion where core Atom packages live, especially if some are consolidated into `atom/atom` and others still live in their own repositories. We will manage this confusion by doing the following: + +- Include folders for _all_ core packages in the `packages` folder of the Atom repo and add README.md files to folders of those packages that still live in separate repos. This will allow us to direct users to the proper home for packages that are not yet consolidated. + +- Archive the repositories for consolidated core packages, but only after migrating existing issues, merging or closing existing PRs, and updating the README.md to point to the new home of the package code. + +Also, contributors will now have to fork, clone, and build `atom/atom` to contribute to core packages where they would previously just need to clone the package repository. This might put added burden on them such as installing necessary build dependencies on their machine that they wouldn't otherwise need. It is very likely we could simplify this process for them, though. + +One final drawback is that it will now be harder to have single-package maintainers. We currently have 7 core packages where there is a maintainer who isn't a part of the core Atom maintainers team. These maintainers generally are able to merge community PRs and make commits to those packages with their own judgement. If we get rid of individual package repositories, do we now make those maintainers full Atom maintainers? + +## Rationale and alternatives + +The Motivation section explains most of the rationale, so this section will focus on the process of consolidating packages back into `atom/atom`. The set of packages we've chosen to consolidate were evaluated based on a few factors: + +- Number of open issues and PRs (exclude any with > 10 open PRs) +- Time since last update (longer duration since last update is prioritized) +- Number of package-only maintainers on the repo (exclude any with package maintainers for now) + +Using this criteria, all 91 packages have been evaluated and categorized to determine whether they are good candidates for consolidation: + +#### Initial Consolidation Candidates + +| Package | Open Issues | Open PRs | Outside Maintainers | Last Updated | +|---------|-------------|----------|---------------------| -------------| +| **[about]** | 2 | 0 | 0 | 7/11/18 | +| **[archive-view]** | 10 | 0 | 0 | 6/3/18 | +| **[atom-dark-syntax]** | 5 | 0 | 0 | 12/6/17 | +| **[atom-dark-ui]** | 1 | 2 | 0 | 2/13/18 | +| **[atom-light-syntax]** | 1 | 0 | 0 | 10/17/16 | +| **[atom-light-ui]** | 1 | 0 | 0 | 2/13/18 | +| **[autoflow]** | 17 | 4 | 0 | 4/17/18 | +| **[autosave]** | 13 | 0 | 0 | 9/16/17 | +| **[background-tips]** | 3 | 2 | 0 | 2/17/18 | +| **[base16-tomorrow-dark-theme]** | 5 | 0 | 0 | 1/10/17 | +| **[base16-tomorrow-light-theme]** | 1 | 0 | 0 | 1/10/17 | +| **[bookmarks]** | 19 | 4 | 0 | 12/10/17 | +| **[bracket-matcher]** | 74 | 8 | 0 | 3/20/18 | +| **[command-palette]** | 18 | 6 | 0 | 2/27/18 | +| **[dalek]** | 2 | 0 | 0 | 2/28/18 | +| **[deprecation-cop]** | 5 | 0 | 0 | 9/7/17 | +| **[dev-live-reload]** | 4 | 0 | 0 | 11/14/17 | +| **[encoding-selector]** | 11 | 2 | 0 | 4/19/18 | +| **[exception-reporting]** | 5 | 0 | 0 | 2/6/18 | +| **[git-diff]** | 38 | 1 | 0 | 1/18/18 | +| **[go-to-line]** | 5 | 2 | 0 | 1/25/18 | +| **[grammar-selector]** | 3 | 1 | 0 | 4/12/18 | +| **[image-view]** | 4 | 4 | 0 | 7/9/18 | +| **[incompatible-packages]** | 1 | 0 | 0 | 4/25/17 | +| **[keybinding-resolver]** | 11 | 3 | 0 | 7/6/18 | +| **[language-clojure]** | 13 | 3 | 0 | 1/26/18 | +| **[language-coffee-script]** | 9 | 2 | 0 | 11/1/17 | +| **[language-csharp]** | 1 | 1 | 0 | 4/27/18 | +| **[language-css]** | 6 | 7 | 0 | 6/11/18 | +| **[language-gfm]** | 52 | 9 | 0 | 6/15/18 | +| **[language-git]** | 4 | 2 | 0 | 4/18/17 | +| **[language-html]** | 11 | 4 | 0 | 7/5/18 | +| **[language-hyperlink]** | 2 | 3 | 0 | 10/25/17 | +| **[language-json]** | 1 | 0 | 0 | 5/11/18 | +| **[language-less]** | 5 | 1 | 0 | 6/11/18 | +| **[language-make]** | 7 | 3 | 0 | 11/26/16 | +| **[language-mustache]** | 0 | 0 | 0 | 2/5/18 | +| **[language-objective-c]** | 2 | 0 | 0 | 12/1/15 | +| **[language-php]** | 25 | 7 | 0 | 6/11/18 | +| **[language-property-list]** | 1 | 0 | 0 | 3/11/17 | +| **[language-python]** | 33 | 4 | 0 | 6/18/18 | +| **[language-ruby]** | 38 | 10 | 0 | 10/25/17 | +| **[language-ruby-on-rails]** | 9 | 6 | 0 | 12/7/17 | +| **[language-sass]** | 12 | 5 | 0 | 5/2/18 | +| **[language-shellscript]** | 12 | 3 | 0 | 6/18/18 | +| **[language-source]** | 0 | 0 | 0 | 1/6/15 | +| **[language-sql]** | 6 | 4 | 0 | 1/26/18 | +| **[language-text]** | 1 | 0 | 0 | 3/9/18 | +| **[language-todo]** | 10 | 6 | 0 | 1/26/18 | +| **[language-toml]** | 1 | 0 | 0 | 1/6/18 | +| **[language-typescript]** | 6 | 0 | 0 | 6/18/18 | +| **[language-xml]** | 2 | 1 | 0 | 6/12/17 | +| **[language-yaml]** | 8 | 2 | 0 | 3/9/18 | +| **[line-ending-selector]** | 10 | 0 | 0 | 5/18/18 | +| **[link]** | 0 | 1 | 0 | 11/14/17 | +| **[metrics]** | 1 | 2 | 0 | 7/5/18 | +| **[notifications]** | 29 | 8 | 0 | 3/22/18 | +| **[one-dark-syntax]** | 4 | 0 | 0 | 5/27/18 | +| **[one-dark-ui]** | 13 | 1 | 0 | 5/1/18 | +| **[one-light-syntax]** | 2 | 1 | 0 | 5/27/18 | +| **[one-light-ui]** | 2 | 0 | 0 | 5/1/18 | +| **[open-on-github]** | 8 | 3 | 0 | 11/21/17 | +| **[package-generator]** | 10 | 2 | 0 | 11/16/17 | +| **[status-bar]** | 25 | 3 | 0 | 11/6/17 | +| **[styleguide]** | 12 | 2 | 0 | 4/12/18 | +| **[tabs]** | 66 | 7 | 0 | 5/13/18 | +| **[timecop]** | 5 | 0 | 0 | 11/4/17 | +| **[update-package-dependencies]** | 0 | 0 | 0 | 12/10/17 | +| **[welcome]** | 0 | 0 | 0 | 11/21/17 | +| **[whitespace]** | 31 | 6 | 0 | 5/30/18 | +| **[wrap-guide]** | 3 | 4 | 0 | 11/27/17 | + +#### Packages Consolidated Later + +The following packages will not be consolidated until the stated reasons can be resolved or we decide on a consolidation strategy for them: + +| Package | Open Issues | Open PRs | Outside Maintainers | Last Updated | Reason | +|---------|-------------|----------|---------------------|--------------|-------| +| **[find-and-replace]** | 219 | 17 | 0 | 6/4/18 | Too many open PRs | +| **[fuzzy-finder]** | 89 | 22 | 0 | 5/17/18 | Too many open PRs | +| **[language-c]** | 53 | 15 | 0 | 7/10/18 | Too many open PRs | +| **[language-go]** | 12 | 2 | **1** | 6/18/18 | Package maintainer, possibly inactive? | +| **[language-java]** | 8 | 2 | **1** | 6/11/18 | Package maintainer | +| **[language-javascript]** | 66 | 12 | 0 | 7/6/18 | Too many open PRs | +| **[language-perl]** | 17 | 1 | **1** | 10/30/17 | Package maintainer, possibly inactive? | +| **[markdown-preview]** | 139 | 12 | 0 | 1/8/18 | Too many open PRs | +| **[settings-view]** | 137 | 18 | 0 | 5/17/18 | Too many open PRs | +| **[snippets]** | 57 | 4 | **1** | 4/17/18 | Package maintainer | +| **[solarized-dark-syntax]** | 8 | 3 | **1** | 5/27/18 | Package maintainer | +| **[solarized-light-syntax]** | 2 | 3 | **1** | 5/27/18 | Package maintainer | +| **[spell-check]** | 68 | 14 | **1** | 5/25/18 | Too many open PRs, package maintainer | +| **[symbols-view]** | 86 | 13 | 0 | 12/10/17 | Too many open PRs | +| **[tree-view]** | 210 | 36 | 0 | 3/21/18 | Too many open PRs | + +#### Packages to Never Consolidate + +These packages will not be consolidated for the following reasons: + +| Package | Open Issues | Open PRs | Outside Maintainers | Last Updated | Reason | +|---------|-------------|----------|---------------------|--------------|-------| +| **[autocomplete-atom-api]** | | | | | Blocks contribution from Facebook | +| **[autocomplete-css]** | | | | | Same as above | +| **[autocomplete-html]** | | | | | Same as above | +| **[autocomplete-plus]** | | | | | Same as above | +| **[autocomplete-snippets]** | | | | | Same as above | +| **[github]** | | | | | Independent project | + +### Consolidation Process + +To consolidate a single core package repository back into `atom/atom`, the following steps will be taken: + +1. All open pull requests on the package's repository must either be closed or merged before consolidation can proceed +1. The package repository's code in `master` will be copied over to a subfolder in Atom's `packages` folder with a subfolder bearing that package's name. +1. A test CI build will be run to ensure that the package loads and works correctly at first glance +1. The package's original repository will have all of its existing issues moved over to `atom/atom` using a bulk issue mover tool +1. The package's original repository will have its README.md to point contributors to the code's new home in `atom/atom` +1. The package's original repository will now be archived + +### Alternative Approaches + +We haven't yet identified another approach which allows us to achieve the goals set forth in this RFC without consolidating these packages into `atom/atom`. + +## Unresolved questions + +- What are the criteria we might use to eventually decide to move larger packages like `tree-view`, `settings-view`, and `find-and-replace` back into `atom/atom`? + +- Is there a good reason to not move the `language-*` packages into `atom/atom`? + +- Will we be losing any useful data about these packages if we don't have standalone repositories anymore? + +[about]: https://github.com/atom/about +[archive-view]: https://github.com/atom/archive-view +[atom-dark-syntax]: https://github.com/atom/atom-dark-syntax +[atom-dark-ui]: https://github.com/atom/atom-dark-ui +[atom-light-syntax]: https://github.com/atom/atom-light-syntax +[atom-light-ui]: https://github.com/atom/atom-light-ui +[autocomplete-atom-api]: https://github.com/atom/autocomplete-atom-api +[autocomplete-css]: https://github.com/atom/autocomplete-css +[autocomplete-html]: https://github.com/atom/autocomplete-html +[autocomplete-plus]: https://github.com/atom/autocomplete-plus +[autocomplete-snippets]: https://github.com/atom/autocomplete-snippets +[autoflow]: https://github.com/atom/autoflow +[autosave]: https://github.com/atom/autosave +[background-tips]: https://github.com/atom/background-tips +[base16-tomorrow-dark-theme]: https://github.com/atom/base16-tomorrow-dark-theme +[base16-tomorrow-light-theme]: https://github.com/atom/base16-tomorrow-light-theme +[bookmarks]: https://github.com/atom/bookmarks +[bracket-matcher]: https://github.com/atom/bracket-matcher +[command-palette]: https://github.com/atom/command-palette +[dalek]: https://github.com/atom/dalek +[deprecation-cop]: https://github.com/atom/deprecation-cop +[dev-live-reload]: https://github.com/atom/dev-live-reload +[encoding-selector]: https://github.com/atom/encoding-selector +[exception-reporting]: https://github.com/atom/exception-reporting +[find-and-replace]: https://github.com/atom/find-and-replace +[fuzzy-finder]: https://github.com/atom/fuzzy-finder +[git-diff]: https://github.com/atom/git-diff +[github]: https://github.com/atom/github +[go-to-line]: https://github.com/atom/go-to-line +[grammar-selector]: https://github.com/atom/grammar-selector +[image-view]: https://github.com/atom/image-view +[incompatible-packages]: https://github.com/atom/incompatible-packages +[keybinding-resolver]: https://github.com/atom/keybinding-resolver +[language-c]: https://github.com/atom/language-c +[language-clojure]: https://github.com/atom/language-clojure +[language-coffee-script]: https://github.com/atom/language-coffee-script +[language-csharp]: https://github.com/atom/language-csharp +[language-css]: https://github.com/atom/language-css +[language-gfm]: https://github.com/atom/language-gfm +[language-git]: https://github.com/atom/language-git +[language-go]: https://github.com/atom/language-go +[language-html]: https://github.com/atom/language-html +[language-hyperlink]: https://github.com/atom/language-hyperlink +[language-java]: https://github.com/atom/language-java +[language-javascript]: https://github.com/atom/language-javascript +[language-json]: https://github.com/atom/language-json +[language-less]: https://github.com/atom/language-less +[language-make]: https://github.com/atom/language-make +[language-mustache]: https://github.com/atom/language-mustache +[language-objective-c]: https://github.com/atom/language-objective-c +[language-perl]: https://github.com/atom/language-perl +[language-php]: https://github.com/atom/language-php +[language-property-list]: https://github.com/atom/language-property-list +[language-python]: https://github.com/atom/language-python +[language-ruby]: https://github.com/atom/language-ruby +[language-ruby-on-rails]: https://github.com/atom/language-ruby-on-rails +[language-sass]: https://github.com/atom/language-sass +[language-shellscript]: https://github.com/atom/language-shellscript +[language-source]: https://github.com/atom/language-source +[language-sql]: https://github.com/atom/language-sql +[language-text]: https://github.com/atom/language-text +[language-todo]: https://github.com/atom/language-todo +[language-toml]: https://github.com/atom/language-toml +[language-typescript]: https://github.com/atom/language-typescript +[language-xml]: https://github.com/atom/language-xml +[language-yaml]: https://github.com/atom/language-yaml +[line-ending-selector]: https://github.com/atom/line-ending-selector +[link]: https://github.com/atom/link +[markdown-preview]: https://github.com/atom/markdown-preview +[metrics]: https://github.com/atom/metrics +[notifications]: https://github.com/atom/notifications +[one-dark-syntax]: https://github.com/atom/one-dark-syntax +[one-dark-ui]: https://github.com/atom/one-dark-ui +[one-light-syntax]: https://github.com/atom/one-light-syntax +[one-light-ui]: https://github.com/atom/one-light-ui +[open-on-github]: https://github.com/atom/open-on-github +[package-generator]: https://github.com/atom/package-generator +[settings-view]: https://github.com/atom/settings-view +[snippets]: https://github.com/atom/snippets +[solarized-dark-syntax]: https://github.com/atom/solarized-dark-syntax +[solarized-light-syntax]: https://github.com/atom/solarized-light-syntax +[spell-check]: https://github.com/atom/spell-check +[status-bar]: https://github.com/atom/status-bar +[styleguide]: https://github.com/atom/styleguide +[symbols-view]: https://github.com/atom/symbols-view +[tabs]: https://github.com/atom/tabs +[timecop]: https://github.com/atom/timecop +[tree-view]: https://github.com/atom/tree-view +[update-package-dependencies]: https://github.com/atom/update-package-dependencies +[welcome]: https://github.com/atom/welcome +[whitespace]: https://github.com/atom/whitespace +[wrap-guide]: https://github.com/atom/wrap-guide From f4ed544646730caefd1a6ea76554d4eaa20552ce Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Thu, 12 Jul 2018 11:12:10 -0400 Subject: [PATCH 02/28] Fix markdown --- docs/rfcs/003-consolidate-core-packages.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md index a96b8d947..5cd3bdbcf 100644 --- a/docs/rfcs/003-consolidate-core-packages.md +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -20,7 +20,7 @@ Let's cover each of the bullet points mentioned above: Imagine that a new contributor wants to add a small new feature to the `tree-view` package. The first place they are likely to look is the `atom/atom` repository. Scanning through the folders will lead to a dead end, nothing that looks like `tree-view` code can be found. They might take one of the following steps next: -- By reading README.md, maybe they will decide to click the link to the Atom Flight Manual and maybe_ find the [Contributing to Official Atom Packages](https://flight-manual.atom.io/hacking-atom/sections/contributing-to-official-atom-packages/) page there. +- By reading README.md, maybe they will decide to click the link to the Atom Flight Manual and _maybe_ find the [Contributing to Official Atom Packages](https://flight-manual.atom.io/hacking-atom/sections/contributing-to-official-atom-packages/) page there. - They could read the CONTRIBUTING.md file which [has a section](https://github.com/atom/atom/blob/master/CONTRIBUTING.md#atom-and-packages) that explains where to find the repos for core packages and how to contribute, but we don't really have a clear pointer to that in our README.md - If they don't happen to find that page, they might use Google to search for "atom tree view" and find the atom/tree-view repo and _maybe_ read the CONTRIBUTING.md file which sends them to Atom's overall contribution documentation - They might go to the Atom Forum or Slack community to ask how to contribute to From 79de6745081c80feaeb9aeac95caa06f8e8b8667 Mon Sep 17 00:00:00 2001 From: Ryan Holinshead Date: Fri, 13 Jul 2018 16:45:04 -0700 Subject: [PATCH 03/28] Currently, the updateClassList function on the TextEditorComponent does not properly re-add its managed classes (editor, is-focused, mini) to the element when the element has been re-rendered with changed classes passed in. This fixes the issue by always adding the newClassList classes to the element and relying on the element.classList.add to determine if the classes already exist (and should be ignored) Released under CC0 --- src/text-editor-component.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index fdb28ce0e..9b30588e0 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -848,10 +848,7 @@ class TextEditorComponent { } for (let i = 0; i < newClassList.length; i++) { - const className = newClassList[i] - if (!oldClassList || !oldClassList.includes(className)) { - this.element.classList.add(className) - } + this.element.classList.add(newClassList[i]) } this.classList = newClassList From e83871fe1b69340bfe71af8904775025c01e33f3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 16 Jul 2018 13:49:30 -0700 Subject: [PATCH 04/28] Update RFC 003 based on feedback and prototype findings --- docs/rfcs/003-consolidate-core-packages.md | 46 ++++++++++++---------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md index 5cd3bdbcf..6212a8d6d 100644 --- a/docs/rfcs/003-consolidate-core-packages.md +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -6,7 +6,7 @@ Proposed ## Summary -Atom's official distribution is comprised of 91 core packages which provide its built-in functionality. These packages currently live in their own independent repositories in the Atom organization, all with their own separate issues, PRs, releases, and CI configurations. This RFC proposes that by consolidating most, if not all, of these core packages back into the `atom/atom` repo, we will see the following benefits: +Atom's official distribution is comprised of 92 core packages which provide its built-in functionality. These packages currently live in their own independent repositories in the Atom organization, all with their own separate issues, PRs, releases, and CI configurations. This RFC proposes that by consolidating most, if not all, of these core packages back into the `atom/atom` repo, we will see the following benefits: - Less confusion for new contributors - Simpler core package contribution experience @@ -18,18 +18,18 @@ Let's cover each of the bullet points mentioned above: ### Less confusion for contributors -Imagine that a new contributor wants to add a small new feature to the `tree-view` package. The first place they are likely to look is the `atom/atom` repository. Scanning through the folders will lead to a dead end, nothing that looks like `tree-view` code can be found. They might take one of the following steps next: +Imagine that a new contributor wants to add a small new feature to the `tree-view` package. The first place they are likely to look is the `atom/atom` repository. Scanning through the folders will lead to a dead end as nothing that looks like `tree-view` code can be found. They might take one of the following steps next: -- By reading README.md, maybe they will decide to click the link to the Atom Flight Manual and _maybe_ find the [Contributing to Official Atom Packages](https://flight-manual.atom.io/hacking-atom/sections/contributing-to-official-atom-packages/) page there. +- By reading README.md, maybe they will decide to click the link to the Atom Flight Manual and _maybe_ find the [Contributing to Official Atom Packages](https://flight-manual.atom.io/hacking-atom/sections/contributing-to-official-atom-packages/) page there - They could read the CONTRIBUTING.md file which [has a section](https://github.com/atom/atom/blob/master/CONTRIBUTING.md#atom-and-packages) that explains where to find the repos for core packages and how to contribute, but we don't really have a clear pointer to that in our README.md - If they don't happen to find that page, they might use Google to search for "atom tree view" and find the atom/tree-view repo and _maybe_ read the CONTRIBUTING.md file which sends them to Atom's overall contribution documentation -- They might go to the Atom Forum or Slack community to ask how to contribute to +- They might go to the Atom Forum or Slack community to ask how to contribute to a particular part of Atom and *hopefully* get a helpful response that points them in the right direction -Having all of the core Atom packages represented in a top-level `packages` folder, even if they don't actually live in the repo, will go a long way to making the core package code more discoverable. +Having all of the core Atom packages represented in a top-level `packages` folder, even if they all don't actually live in the repo, will go a long way to making the core package code more discoverable. ### Simpler core package contribution experience -Separating core Atom features out into separate repositories and delivered via `apm` is a great idea in theory because it validates the Atom package ecosystem and gives developers many examples of how to develop an Atom package. It also gives Atom developers real-world experience working with Atom's APIs so that we ensure community package authors have the same hackability that the Atom developers enjoy. +Separating core Atom features out into individual repositories and delivering them to Atom builds via `apm` is a great idea in theory because it validates the Atom package ecosystem and gives developers many examples of how to develop an Atom package. It also gives Atom developers real-world experience working with Atom's APIs so that we ensure community package authors have the same hackability that Atom developers enjoy. On the other hand, having these packages live in separate repositories and released "independently" introduces a great deal of overhead when adding new features. Here is a comparison of the current package development workflow contrasted to what we could achieve with consolidated packages: @@ -43,15 +43,13 @@ For example, to add a single feature to the `tree-view` package, one must: 1. Open a PR to the `tree-view` repo and wait for CI to pass and a maintainer to review it 1. Work with maintainers to get the PR approved and merged -After this is finished, an Atom maintainer must take the following steps +After this is finished, an Atom maintainer must take the following steps: 1. Clone the `tree-view` repo 2. Run `apm publish` to publish a new release of the package 3. Edit `package.json` in the Atom repo to reflect the new version of `tree-view` 4. Commit and push the changes to the relevant branch where the change belongs (`master` or `1.nn-releases`) -If `tree-view` was moved into the `atom/atom` repository - #### Simplified Package Development If we were to move `tree-view` (or any other core Atom package) back into `atom/atom`, the development workflow would look more like this: @@ -66,7 +64,7 @@ At this point, the change is merged into Atom and ready for inclusion in the nex ### Greatly reduced burden for maintainers -Since packages all have their own repositories, this means that we have to watch 91 different repos for issues and pull requests. This also means that we have to redirect issues filed on `atom/atom` to the appropriate repository when a user doesn't know where it belongs. Even more importantly, there's not an easy way to prioritize and track issues across the Atom organization without using GitHub projects. +Since packages all have their own repositories, this means that we have to watch 91 different repos for issues and pull requests. This also means that we have to redirect issues filed on `atom/atom` to the appropriate repository when a user doesn't know where it belongs. Even more importantly, there's not an easy way to prioritize and track issues across the Atom organization without using GitHub Projects. Also, as mentioned above, there's the added duty of doing the package "version dance" when we merge any new PRs to a package repository: publish the package update, update `package.json` in Atom. It's very easy to forget to do this and not have community contributions included in the next Atom release! @@ -74,13 +72,15 @@ The more core packages live in `atom/atom`, the less work Atom maintainers have ## Explanation -Many of Atom's core packages now live in the core `atom/atom` repository. To the Atom user, this change will be imperceptible as these packages still show up in the list of Core Packages in the Settings View. For maintainers and contributors, there will be less juggling of repositories and no more publishing of updates to these packages with `apm`. +Many of Atom's core packages now live in the core `atom/atom` repository. To the Atom user, this change will be imperceptible as these packages still show up in the list of Core Packages in the Settings View. Users can still disable these packages if they want to replace their behavior with other packages. -Contributors now clone and build `atom/atom` to work on improvements to core packages. They will no longer have to use `apm link` in dev mode to test changes they make to packages in the repo's `packages` folder. +For maintainers and contributors, there will be less juggling of repositories and no more publishing of updates to these packages with `apm`: + +Contributors now clone and build `atom/atom` to work on improvements to core packages. They will no longer have to use `apm link` in dev mode to test changes they make to packages in the repo's `packages` folder. Core packages that aren't consolidated still have folders under `packages` with README.md files that point to the home repository for that package. When a contributor sends a PR to `atom/atom` that only affects files in a folder under `packages`, only the specs for the relevant package folders will be executed using Atom's CI scripts. This means that a full Atom build will not be required when no Atom Core code is changed in a PR. Package specs are also now run against all 3 OSes on Atom `master` and release builds. -Core packages that aren't consolidated still have folders under `packages` with README.md files that point to the home repository for that package. +Atom maintainers no longer have to publish new versions to consolidated core packages and then edit `package.json` to bump the package version in a particular Atom release branch (Stable, Beta, or `master`). When a PR against a consolidated core package in `atom/atom` is merged, no version number change is required and the changes will immediately be a part of the next release from that branch. ## Drawbacks @@ -180,7 +180,7 @@ Using this criteria, all 91 packages have been evaluated and categorized to dete | **[whitespace]** | 31 | 6 | 0 | 5/30/18 | | **[wrap-guide]** | 3 | 4 | 0 | 11/27/17 | -#### Packages Consolidated Later +#### Packages to be Consolidated Later The following packages will not be consolidated until the stated reasons can be resolved or we decide on a consolidation strategy for them: @@ -220,15 +220,21 @@ These packages will not be consolidated for the following reasons: To consolidate a single core package repository back into `atom/atom`, the following steps will be taken: 1. All open pull requests on the package's repository must either be closed or merged before consolidation can proceed -1. The package repository's code in `master` will be copied over to a subfolder in Atom's `packages` folder with a subfolder bearing that package's name. -1. A test CI build will be run to ensure that the package loads and works correctly at first glance -1. The package's original repository will have all of its existing issues moved over to `atom/atom` using a bulk issue mover tool -1. The package's original repository will have its README.md to point contributors to the code's new home in `atom/atom` -1. The package's original repository will now be archived +1. The package repository's code in `master` will be copied over to Atom's `packages` folder in a subfolder bearing that package's name. +1. Atom's `package.json` file will be updated to change the package's `packageDependencies` entry to reference its local path with the following syntax: `"tree-view": "file:./packages/tree-view"` +1. A test build will be created to manually verify the package loads and works correctly at first glance and also that package specs pass +1. A PR will be sent to `atom/atom` to verify that CI passes with the introduction of the consolidated package +1. Once CI is clean and the PR is approved, the PR will be merged +1. The package's original repository will have all of its existing issues moved over to `atom/atom` using a bulk issue mover tool, assigning a label to those issues relative to the package name, like `packages/tree-view` +1. The package's original repository will have its README.md updated to point contributors to the code's new home in `atom/atom` +1. The package's original repository will now be archived on GitHub ### Alternative Approaches -We haven't yet identified another approach which allows us to achieve the goals set forth in this RFC without consolidating these packages into `atom/atom`. +One alternative approach would be to break this core Atom functionality out of packages and put it directly in the Atom codebase without treating them as packages. This would simplify the development process even further but with the following drawbacks: + +- The Atom team would have less regular exposure to Atom package development +- Users would no longer be able to disable core packages to replace their behavior with other packages (different tree views, etc) ## Unresolved questions From a280019847c9863b8926c27888c0f5870eedf3fe Mon Sep 17 00:00:00 2001 From: Ryan Holinshead Date: Mon, 16 Jul 2018 14:58:39 -0700 Subject: [PATCH 05/28] Add spec to test that updateClassList does not blow away the managed class names (editor, is-focused, mini) when the element class names are changed. Released under CC0 --- spec/text-editor-component-spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index cc4c1854a..12c29e2a3 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -818,6 +818,18 @@ describe('TextEditorComponent', () => { expect(element.className).toBe('editor a b') }) + it('does not blow away class names managed by the component when packages change the element class name', async () => { + assertDocumentFocused() + const {component, element, editor} = buildComponent({mini: true}) + element.classList.add('a', 'b') + element.focus() + await component.getNextUpdatePromise() + expect(element.className).toBe('editor mini a b is-focused') + element.className = 'a c d'; + await component.getNextUpdatePromise() + expect(element.className).toBe('a c d editor is-focused mini') + }) + it('ignores resize events when the editor is hidden', async () => { const {component, element, editor} = buildComponent({autoHeight: false}) element.style.height = 5 * component.getLineHeight() + 'px' From 0006249c88c50a8fad76a33c6924d129b85243b3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 18 Jul 2018 14:36:02 -0700 Subject: [PATCH 06/28] :memo: More updates from feedback --- docs/rfcs/003-consolidate-core-packages.md | 33 +++++++++++++--------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md index 6212a8d6d..6e089dae2 100644 --- a/docs/rfcs/003-consolidate-core-packages.md +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -72,7 +72,7 @@ The more core packages live in `atom/atom`, the less work Atom maintainers have ## Explanation -Many of Atom's core packages now live in the core `atom/atom` repository. To the Atom user, this change will be imperceptible as these packages still show up in the list of Core Packages in the Settings View. Users can still disable these packages if they want to replace their behavior with other packages. +Many of Atom's core packages now live in the core `atom/atom` repository. To the Atom user, this change will be imperceptible as these packages still show up in the list of Core Packages in the Settings View. Users can still optionally disable these packages. For maintainers and contributors, there will be less juggling of repositories and no more publishing of updates to these packages with `apm`: @@ -86,7 +86,7 @@ Atom maintainers no longer have to publish new versions to consolidated core pac One possible drawback of this approach is that there might be some initial confusion where core Atom packages live, especially if some are consolidated into `atom/atom` and others still live in their own repositories. We will manage this confusion by doing the following: -- Include folders for _all_ core packages in the `packages` folder of the Atom repo and add README.md files to folders of those packages that still live in separate repos. This will allow us to direct users to the proper home for packages that are not yet consolidated. +- Include a `README.md` file in the `packages` folder which lists core packages that are not consolidated in the Atom repo. This will enable users to find the home repositories of those packages. - Archive the repositories for consolidated core packages, but only after migrating existing issues, merging or closing existing PRs, and updating the README.md to point to the new home of the package code. @@ -188,6 +188,7 @@ The following packages will not be consolidated until the stated reasons can be |---------|-------------|----------|---------------------|--------------|-------| | **[find-and-replace]** | 219 | 17 | 0 | 6/4/18 | Too many open PRs | | **[fuzzy-finder]** | 89 | 22 | 0 | 5/17/18 | Too many open PRs | +| **[github]** | | | | | Independent project | | **[language-c]** | 53 | 15 | 0 | 7/10/18 | Too many open PRs | | **[language-go]** | 12 | 2 | **1** | 6/18/18 | Package maintainer, possibly inactive? | | **[language-java]** | 8 | 2 | **1** | 6/11/18 | Package maintainer | @@ -204,16 +205,13 @@ The following packages will not be consolidated until the stated reasons can be #### Packages to Never Consolidate -These packages will not be consolidated for the following reasons: +These packages will not be consolidated because they would inhibit contributions from our friends in the Nuclide team at Facebook: -| Package | Open Issues | Open PRs | Outside Maintainers | Last Updated | Reason | -|---------|-------------|----------|---------------------|--------------|-------| -| **[autocomplete-atom-api]** | | | | | Blocks contribution from Facebook | -| **[autocomplete-css]** | | | | | Same as above | -| **[autocomplete-html]** | | | | | Same as above | -| **[autocomplete-plus]** | | | | | Same as above | -| **[autocomplete-snippets]** | | | | | Same as above | -| **[github]** | | | | | Independent project | +- **[autocomplete-atom-api]** +- **[autocomplete-css]** +- **[autocomplete-html]** +- **[autocomplete-plus]** +- **[autocomplete-snippets]** ### Consolidation Process @@ -222,7 +220,8 @@ To consolidate a single core package repository back into `atom/atom`, the follo 1. All open pull requests on the package's repository must either be closed or merged before consolidation can proceed 1. The package repository's code in `master` will be copied over to Atom's `packages` folder in a subfolder bearing that package's name. 1. Atom's `package.json` file will be updated to change the package's `packageDependencies` entry to reference its local path with the following syntax: `"tree-view": "file:./packages/tree-view"` -1. A test build will be created to manually verify the package loads and works correctly at first glance and also that package specs pass +1. A test build will be created locally to manually verify that the package loads and works correctly at first glance +1. The package specs for the newly-consolidated package will be run against the local Atom build 1. A PR will be sent to `atom/atom` to verify that CI passes with the introduction of the consolidated package 1. Once CI is clean and the PR is approved, the PR will be merged 1. The package's original repository will have all of its existing issues moved over to `atom/atom` using a bulk issue mover tool, assigning a label to those issues relative to the package name, like `packages/tree-view` @@ -238,12 +237,18 @@ One alternative approach would be to break this core Atom functionality out of p ## Unresolved questions -- What are the criteria we might use to eventually decide to move larger packages like `tree-view`, `settings-view`, and `find-and-replace` back into `atom/atom`? - - Is there a good reason to not move the `language-*` packages into `atom/atom`? + One concern here is that there exist projects which depend directly on these repositories for the TextMate syntax grammars they contain. Moving the code into `atom/atom` would require that we notify the consumers of the grammars so that they can redirect their requests to the `atom/atom` repo. + +- What are the criteria we might use to eventually decide to move larger packages like `tree-view`, `settings-view`, and `find-and-replace` back into `atom/atom`? + - Will we be losing any useful data about these packages if we don't have standalone repositories anymore? +- Should we use `git subtree` to migrate the entire commit history of these packages over or just depend on the history from a package's original repository? + +- Should we use this as an opportunity to remove any unnecessary packages from the core Atom distribution? + [about]: https://github.com/atom/about [archive-view]: https://github.com/atom/archive-view [atom-dark-syntax]: https://github.com/atom/atom-dark-syntax From 84b2ba742db1792b0b8846e84dc5a283eba4bbe4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 18 Jul 2018 16:21:33 -0700 Subject: [PATCH 07/28] :arrow_up: tree-sitter, language packages --- package.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 3ae28ec36..548810948 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "sinon": "1.17.4", "temp": "^0.8.3", "text-buffer": "13.14.5", - "tree-sitter": "0.12.20", + "tree-sitter": "0.13.0", "typescript-simple": "1.0.0", "underscore-plus": "^1.6.8", "winreg": "^1.2.1", @@ -138,18 +138,18 @@ "welcome": "0.36.6", "whitespace": "0.37.6", "wrap-guide": "0.40.3", - "language-c": "0.59.11", + "language-c": "0.60.0", "language-clojure": "0.22.7", "language-coffee-script": "0.49.3", "language-csharp": "1.0.4", "language-css": "0.42.11", "language-gfm": "0.90.5", "language-git": "0.19.1", - "language-go": "0.45.4", - "language-html": "0.50.3", + "language-go": "0.46.0", + "language-html": "0.51.0", "language-hyperlink": "0.16.3", "language-java": "0.30.0", - "language-javascript": "0.128.11", + "language-javascript": "0.129.0", "language-json": "0.19.2", "language-less": "0.34.2", "language-make": "0.22.3", @@ -158,17 +158,17 @@ "language-perl": "0.38.1", "language-php": "0.44.0", "language-property-list": "0.9.1", - "language-python": "0.50.1", + "language-python": "0.51.0", "language-ruby": "0.71.4", "language-ruby-on-rails": "0.25.3", "language-sass": "0.62.0", - "language-shellscript": "0.26.6", + "language-shellscript": "0.27.0", "language-source": "0.9.0", "language-sql": "0.25.10", "language-text": "0.7.4", "language-todo": "0.29.4", "language-toml": "0.18.2", - "language-typescript": "0.3.4", + "language-typescript": "0.4.0", "language-xml": "0.35.2", "language-yaml": "0.32.0" }, From c9a4bb4245f1252fb30e1ea1cf646ac5374a26d7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 18 Jul 2018 17:09:47 -0700 Subject: [PATCH 08/28] Fix logic for updating foldable range cache --- src/tree-sitter-language-mode.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index 7d0377f28..4087cd2a1 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -41,15 +41,12 @@ class TreeSitterLanguageMode { this.emitRangeUpdate = this.emitRangeUpdate.bind(this) this.subscription = this.buffer.onDidChangeText(({changes}) => { - for (let i = changes.length - 1; i >= 0; i--) { + for (let i = 0, {length} = changes; i < length; i++) { const {oldRange, newRange} = changes[i] - const startRow = oldRange.start.row - const oldEndRow = oldRange.end.row - const newEndRow = newRange.end.row this.isFoldableCache.splice( - startRow, - oldEndRow - startRow, - ...new Array(newEndRow - startRow) + newRange.start.row, + oldRange.end.row - oldRange.start.row, + ...new Array(newRange.end.row - newRange.start.row) ) } From 9c9cb120813557eb626bb373ea4bd23fac9425f4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 10:53:43 -0700 Subject: [PATCH 09/28] Use correct executable name for Atom Dev.app in atom.sh --- atom.sh | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/atom.sh b/atom.sh index a1c96abeb..935204bfc 100755 --- a/atom.sh +++ b/atom.sh @@ -79,14 +79,20 @@ if [ $OS == 'Mac' ]; then ATOM_APP_NAME="$(basename "$ATOM_APP")" fi - if [ "$CHANNEL" == 'beta' ]; then - ATOM_EXECUTABLE_NAME="Atom Beta" - elif [ "$CHANNEL" == 'nightly' ]; then - ATOM_EXECUTABLE_NAME="Atom Nightly" - elif [ "$CHANNEL" == 'dev' ]; then - ATOM_EXECUTABLE_NAME="Atom Dev" + if [ ! -z "${ATOM_APP_NAME}" ]; then + # If ATOM_APP_NAME is known, use it as the executable name + ATOM_EXECUTABLE_NAME="${ATOM_APP_NAME%.*}" else - ATOM_EXECUTABLE_NAME="Atom" + # Else choose it from the inferred channel name + if [ "$CHANNEL" == 'beta' ]; then + ATOM_EXECUTABLE_NAME="Atom Beta" + elif [ "$CHANNEL" == 'nightly' ]; then + ATOM_EXECUTABLE_NAME="Atom Nightly" + elif [ "$CHANNEL" == 'dev' ]; then + ATOM_EXECUTABLE_NAME="Atom Dev" + else + ATOM_EXECUTABLE_NAME="Atom" + fi fi if [ -z "${ATOM_PATH}" ]; then From c40af117338829edd1f05a94c74ed1003cfa0804 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 15:34:58 -0700 Subject: [PATCH 10/28] Add current decision to question about using `git subtree` in migrations --- docs/rfcs/003-consolidate-core-packages.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md index 6e089dae2..46b614514 100644 --- a/docs/rfcs/003-consolidate-core-packages.md +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -241,12 +241,14 @@ One alternative approach would be to break this core Atom functionality out of p One concern here is that there exist projects which depend directly on these repositories for the TextMate syntax grammars they contain. Moving the code into `atom/atom` would require that we notify the consumers of the grammars so that they can redirect their requests to the `atom/atom` repo. +- Should we use `git subtree` to migrate the entire commit history of these packages over or just depend on the history from a package's original repository? + + For now, we won't use `git subtree` due to the possibility that bringing over thousands of commits could cause unknown problems in the Atom repo. We may try this for newly consolidated packages in the future if we decide that not having the package commit history is a sufficient impediment to problem investigations. + - What are the criteria we might use to eventually decide to move larger packages like `tree-view`, `settings-view`, and `find-and-replace` back into `atom/atom`? - Will we be losing any useful data about these packages if we don't have standalone repositories anymore? -- Should we use `git subtree` to migrate the entire commit history of these packages over or just depend on the history from a package's original repository? - - Should we use this as an opportunity to remove any unnecessary packages from the core Atom distribution? [about]: https://github.com/atom/about From 3e4acb677f7ab1fe3bb4dc269685d1e091a596f7 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Thu, 19 Jul 2018 15:35:20 -0700 Subject: [PATCH 11/28] Set RFC 003 status to Accepted --- docs/rfcs/003-consolidate-core-packages.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rfcs/003-consolidate-core-packages.md b/docs/rfcs/003-consolidate-core-packages.md index 46b614514..960ac95ce 100644 --- a/docs/rfcs/003-consolidate-core-packages.md +++ b/docs/rfcs/003-consolidate-core-packages.md @@ -2,7 +2,7 @@ ## Status -Proposed +Accepted ## Summary From 94425ebc447cc56153fefcdb3e35a57009ba0b35 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 19 Jul 2018 15:56:38 -0700 Subject: [PATCH 12/28] :arrow_up: language-ruby --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 548810948..b4a257f33 100644 --- a/package.json +++ b/package.json @@ -159,7 +159,7 @@ "language-php": "0.44.0", "language-property-list": "0.9.1", "language-python": "0.51.0", - "language-ruby": "0.71.4", + "language-ruby": "0.72.0", "language-ruby-on-rails": "0.25.3", "language-sass": "0.62.0", "language-shellscript": "0.27.0", From 9136909f0b55a5677b730054ed23236b4e11f4ec Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 19 Jul 2018 16:03:29 -0700 Subject: [PATCH 13/28] :arrow_up: language-html --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b4a257f33..72661dc39 100644 --- a/package.json +++ b/package.json @@ -146,7 +146,7 @@ "language-gfm": "0.90.5", "language-git": "0.19.1", "language-go": "0.46.0", - "language-html": "0.51.0", + "language-html": "0.51.1", "language-hyperlink": "0.16.3", "language-java": "0.30.0", "language-javascript": "0.129.0", From a283ca365fee6e87800a5ced83f67b49ae1bd146 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 19 Jul 2018 17:15:04 -0700 Subject: [PATCH 14/28] Fix a ruby folding issue --- spec/tree-sitter-language-mode-spec.js | 92 +++++++++++++++++++++++++- src/tree-sitter-grammar.js | 34 ++++++++++ src/tree-sitter-language-mode.js | 49 ++++++-------- 3 files changed, 146 insertions(+), 29 deletions(-) diff --git a/spec/tree-sitter-language-mode-spec.js b/spec/tree-sitter-language-mode-spec.js index c23849d30..4bedbd426 100644 --- a/spec/tree-sitter-language-mode-spec.js +++ b/spec/tree-sitter-language-mode-spec.js @@ -12,6 +12,7 @@ const pythonGrammarPath = require.resolve('language-python/grammars/tree-sitter- const jsGrammarPath = require.resolve('language-javascript/grammars/tree-sitter-javascript.cson') const htmlGrammarPath = require.resolve('language-html/grammars/tree-sitter-html.cson') const ejsGrammarPath = require.resolve('language-html/grammars/tree-sitter-ejs.cson') +const rubyGrammarPath = require.resolve('language-ruby/grammars/tree-sitter-ruby.cson') describe('TreeSitterLanguageMode', () => { let editor, buffer @@ -674,7 +675,7 @@ describe('TreeSitterLanguageMode', () => { expect(getDisplayText(editor)).toBe(dedent ` module.exports = class A { - getB (…) { + getB (c,…) { return this.f(g) } } @@ -684,7 +685,7 @@ describe('TreeSitterLanguageMode', () => { expect(getDisplayText(editor)).toBe(dedent ` module.exports = class A { - getB (…) {…} + getB (c,…) {…} } `) }) @@ -942,6 +943,93 @@ describe('TreeSitterLanguageMode', () => { `) }) + it('can target named vs anonymous nodes as fold boundaries', async () => { + const grammar = new TreeSitterGrammar(atom.grammars, rubyGrammarPath, { + parser: 'tree-sitter-ruby', + folds: [ + { + type: 'elsif', + start: {index: 1}, + + // There are no double quotes around the `elsif` type. This indicates + // that we're targeting a *named* node in the syntax tree. The fold + // should end at the nested `elsif` node, not at the token that represents + // the literal string "elsif". + end: {type: ['else', 'elsif']} + }, + { + type: 'else', + + // There are double quotes around the `else` type. This indicates that + // we're targetting an *anonymous* node in the syntax tree. The fold + // should start at the token representing the literal string "else", + // not at an `else` node. + start: {type: '"else"'} + } + ] + }) + + buffer.setText(dedent ` + if a + b + elsif c + d + elsif e + f + else + g + end + `) + + const languageMode = new TreeSitterLanguageMode({buffer, grammar}) + buffer.setLanguageMode(languageMode) + await nextHighlightingUpdate(languageMode) + + expect(languageMode.tree.rootNode.toString()).toBe( + "(program (if (identifier) " + + "(identifier) " + + "(elsif (identifier) " + + "(identifier) " + + "(elsif (identifier) " + + "(identifier) " + + "(else " + + "(identifier))))))" + ) + + editor.foldBufferRow(2) + expect(getDisplayText(editor)).toBe(dedent ` + if a + b + elsif c… + elsif e + f + else + g + end + `) + + editor.foldBufferRow(4) + expect(getDisplayText(editor)).toBe(dedent ` + if a + b + elsif c… + elsif e… + else + g + end + `) + + editor.foldBufferRow(6) + expect(getDisplayText(editor)).toBe(dedent ` + if a + b + elsif c… + elsif e… + else… + end + `) + }) + describe('when folding a node that ends with a line break', () => { it('ends the fold at the end of the previous line', async () => { const grammar = new TreeSitterGrammar(atom.grammars, pythonGrammarPath, { diff --git a/src/tree-sitter-grammar.js b/src/tree-sitter-grammar.js index acea24213..04ca7f438 100644 --- a/src/tree-sitter-grammar.js +++ b/src/tree-sitter-grammar.js @@ -13,6 +13,7 @@ class TreeSitterGrammar { if (params.injectionRegExp) this.injectionRegExp = new RegExp(params.injectionRegExp) this.folds = params.folds || [] + this.folds.forEach(normalizeFoldSpecification) this.commentStrings = { commentStartString: params.comments && params.comments.start, @@ -72,3 +73,36 @@ class TreeSitterGrammar { if (this.registration) this.registration.dispose() } } + +const NODE_NAME_REGEX = /[\w_]+/ + +function matcherForSpec (spec) { + if (typeof spec === 'string') { + if (spec[0] === '"' && spec[spec.length - 1] === '"') { + return { + type: spec.substr(1, spec.length - 2), + named: false + } + } + + if (!NODE_NAME_REGEX.test(spec)) { + return {type: spec, named: false} + } + + return {type: spec, named: true} + } + return spec +} + +function normalizeFoldSpecification (spec) { + if (spec.type) { + if (Array.isArray(spec.type)) { + spec.matchers = spec.type.map(matcherForSpec) + } else { + spec.matchers = [matcherForSpec(spec.type)] + } + } + + if (spec.start) normalizeFoldSpecification(spec.start) + if (spec.end) normalizeFoldSpecification(spec.end) +} diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index 4087cd2a1..feb2c9217 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -269,42 +269,32 @@ class TreeSitterLanguageMode { } getFoldableRangeForNode (node, grammar, existenceOnly) { - const {children, type: nodeType} = node + const {children} = node const childCount = children.length - let childTypes for (var i = 0, {length} = grammar.folds; i < length; i++) { - const foldEntry = grammar.folds[i] + const foldSpec = grammar.folds[i] - if (foldEntry.type) { - if (typeof foldEntry.type === 'string') { - if (foldEntry.type !== nodeType) continue - } else { - if (!foldEntry.type.includes(nodeType)) continue - } - } + if (foldSpec.matchers && !hasMatchingFoldSpec(foldSpec.matchers, node)) continue let foldStart - const startEntry = foldEntry.start + const startEntry = foldSpec.start if (startEntry) { + let foldStartNode if (startEntry.index != null) { - const child = children[startEntry.index] - if (!child || (startEntry.type && startEntry.type !== child.type)) continue - foldStart = child.endPosition + foldStartNode = children[startEntry.index] + if (!foldStartNode || startEntry.matchers && !hasMatchingFoldSpec(startEntry.matchers, foldStartNode)) continue } else { - if (!childTypes) childTypes = children.map(child => child.type) - const index = typeof startEntry.type === 'string' - ? childTypes.indexOf(startEntry.type) - : childTypes.findIndex(type => startEntry.type.includes(type)) - if (index === -1) continue - foldStart = children[index].endPosition + foldStartNode = children.find(child => hasMatchingFoldSpec(startEntry.matchers, child)) + if (!foldStartNode) continue } + foldStart = new Point(foldStartNode.endPosition.row, Infinity) } else { foldStart = new Point(node.startPosition.row, Infinity) } let foldEnd - const endEntry = foldEntry.end + const endEntry = foldSpec.end if (endEntry) { let foldEndNode if (endEntry.index != null) { @@ -312,12 +302,8 @@ class TreeSitterLanguageMode { foldEndNode = children[index] if (!foldEndNode || (endEntry.type && endEntry.type !== foldEndNode.type)) continue } else { - if (!childTypes) childTypes = children.map(foldEndNode => foldEndNode.type) - const index = typeof endEntry.type === 'string' - ? childTypes.indexOf(endEntry.type) - : childTypes.findIndex(type => endEntry.type.includes(type)) - if (index === -1) continue - foldEndNode = children[index] + foldEndNode = children.find(child => hasMatchingFoldSpec(endEntry.matchers, child)) + if (!foldEndNode) continue } if (foldEndNode.endIndex - foldEndNode.startIndex > 1 && foldEndNode.startPosition.row > foldStart.row) { @@ -768,7 +754,12 @@ class LayerHighlightIterator { } else { this.atEnd = false this.openTags.push(id) + const {startIndex} = this.treeCursor while (this.treeCursor.gotoFirstChild()) { + if (this.treeCursor.startIndex > startIndex) { + this.treeCursor.gotoParent() + break + } this.containingNodeTypes.push(this.treeCursor.nodeType) this.containingNodeChildIndices.push(0) const scopeName = this.currentScopeName() @@ -1041,6 +1032,10 @@ function last (array) { return array[array.length - 1] } +function hasMatchingFoldSpec (specs, node) { + return specs.some(({type, named}) => type === node.type && named === node.isNamed) +} + // TODO: Remove this once TreeSitterLanguageMode implements its own auto-indent system. [ '_suggestedIndentForLineWithScopeAtBufferRow', From 137acb2f904d4e67701353150ea2de3d454db666 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 10:15:30 -0700 Subject: [PATCH 15/28] Fix highlighting when parent nodes extend beyond their first and last children --- spec/tree-sitter-language-mode-spec.js | 32 +++++ src/tree-sitter-language-mode.js | 172 +++++++++++++------------ 2 files changed, 124 insertions(+), 80 deletions(-) diff --git a/spec/tree-sitter-language-mode-spec.js b/spec/tree-sitter-language-mode-spec.js index 4bedbd426..22d80b823 100644 --- a/spec/tree-sitter-language-mode-spec.js +++ b/spec/tree-sitter-language-mode-spec.js @@ -314,6 +314,38 @@ describe('TreeSitterLanguageMode', () => { ]) }) + it('handles nodes that start before their first child and end after their last child', async () => { + const grammar = new TreeSitterGrammar(atom.grammars, rubyGrammarPath, { + parser: 'tree-sitter-ruby', + scopes: { + 'bare_string': 'string', + 'interpolation': 'embedded', + '"#{"': 'punctuation', + '"}"': 'punctuation', + } + }) + + // The bare string node `bc#{d}ef` has one child: the interpolation, and that child + // starts later and ends earlier than the bare string. + buffer.setText('a = %W( bc#{d}ef )') + + const languageMode = new TreeSitterLanguageMode({buffer, grammar}) + buffer.setLanguageMode(languageMode) + await nextHighlightingUpdate(languageMode) + + expectTokensToEqual(editor, [ + [ + {text: 'a = %W( ', scopes: []}, + {text: 'bc', scopes: ['string']}, + {text: '#{', scopes: ['string', 'embedded', 'punctuation']}, + {text: 'd', scopes: ['string', 'embedded']}, + {text: '}', scopes: ['string', 'embedded', 'punctuation']}, + {text: 'ef', scopes: ['string']}, + {text: ' )', scopes: []}, + ] + ]) + }) + describe('when the buffer changes during a parse', () => { it('immediately parses again when the current parse completes', async () => { const grammar = new TreeSitterGrammar(atom.grammars, jsGrammarPath, { diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index feb2c9217..b8a400ce7 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -697,8 +697,14 @@ class HighlightIterator { iterator.languageLayer.tree.rootNode.endPosition ).toString() ) + console.log('close', iterator.closeTags.map(id => this.shortClassNameForScopeId(id))) + console.log('open', iterator.openTags.map(id => this.shortClassNameForScopeId(id))) } } + + shortClassNameForScopeId (id) { + return this.languageMode.classNameForScopeId(id).replace(/syntax--/g, '') + } } class LayerHighlightIterator { @@ -745,28 +751,15 @@ class LayerHighlightIterator { this.containingNodeChildIndices.push(childIndex) this.containingNodeEndIndices.push(this.treeCursor.endIndex) - const scopeName = this.currentScopeName() - if (scopeName) { - const id = this.idForScope(scopeName) + const scopeId = this._currentScopeId() + if (scopeId) { if (this.treeCursor.startIndex < targetIndex) { - insertContainingTag(id, this.treeCursor.startIndex, containingTags, containingTagStartIndices) + insertContainingTag(scopeId, this.treeCursor.startIndex, containingTags, containingTagStartIndices) containingTagEndIndices.push(this.treeCursor.endIndex) } else { this.atEnd = false - this.openTags.push(id) - const {startIndex} = this.treeCursor - while (this.treeCursor.gotoFirstChild()) { - if (this.treeCursor.startIndex > startIndex) { - this.treeCursor.gotoParent() - break - } - this.containingNodeTypes.push(this.treeCursor.nodeType) - this.containingNodeChildIndices.push(0) - const scopeName = this.currentScopeName() - if (scopeName) { - this.openTags.push(this.idForScope(scopeName)) - } - } + this.openTags.push(scopeId) + this._moveDown() break } } @@ -793,71 +786,31 @@ class LayerHighlightIterator { this.closeTags.length = 0 this.openTags.length = 0 - if (this.done) return - - while (true) { + while (!(this.done || (didMove && (this.closeTags.length || this.openTags.length)))) { if (this.atEnd) { - if (this.treeCursor.gotoNextSibling()) { + if (this._moveRight()) { + const scopeId = this._currentScopeId() + if (scopeId) this.openTags.push(scopeId) + didMove = true this.atEnd = false - const depth = this.containingNodeTypes.length - this.containingNodeTypes[depth - 1] = this.treeCursor.nodeType - this.containingNodeChildIndices[depth - 1]++ - this.containingNodeEndIndices[depth - 1] = this.treeCursor.endIndex - - while (true) { - const {startIndex} = this.treeCursor - const scopeName = this.currentScopeName() - if (scopeName) { - this.openTags.push(this.idForScope(scopeName)) - } - - if (this.treeCursor.gotoFirstChild()) { - if ((this.closeTags.length || this.openTags.length) && - this.treeCursor.startIndex > startIndex) { - this.treeCursor.gotoParent() - break - } - - this.containingNodeTypes.push(this.treeCursor.nodeType) - this.containingNodeChildIndices.push(0) - this.containingNodeEndIndices.push(this.treeCursor.endIndex) - } else { - break - } - } - } else if (this.treeCursor.gotoParent()) { - this.atEnd = false - this.containingNodeTypes.pop() - this.containingNodeChildIndices.pop() - this.containingNodeEndIndices.pop() + this._moveDown() + } else if (this._moveUp(true)) { + didMove = true + this.atEnd = true } else { this.done = true - break } - } else { - this.atEnd = true + } else if (this._moveDown()) { didMove = true + } else { + const scopeId = this._currentScopeId() + if (scopeId) this.closeTags.push(scopeId) - const scopeName = this.currentScopeName() - if (scopeName) { - this.closeTags.push(this.idForScope(scopeName)) - } - - const endIndex = this.treeCursor.endIndex - let depth = this.containingNodeEndIndices.length - while (depth > 1 && this.containingNodeEndIndices[depth - 2] === endIndex) { - this.treeCursor.gotoParent() - this.containingNodeTypes.pop() - this.containingNodeChildIndices.pop() - this.containingNodeEndIndices.pop() - --depth - const scopeName = this.currentScopeName() - if (scopeName) this.closeTags.push(this.idForScope(scopeName)) - } + didMove = true + this.atEnd = true + this._moveUp(false) } - - if (didMove && (this.closeTags.length || this.openTags.length)) break } } @@ -891,16 +844,75 @@ class LayerHighlightIterator { // Private methods - currentScopeName () { - return this.languageLayer.grammar.scopeMap.get( + _moveUp (atLastChild) { + let result = false + const {endIndex} = this.treeCursor + let depth = this.containingNodeEndIndices.length + while (depth > 1) { + // Once the iterator has found a scope boundary, it needs to stay at the same + // position, so it should not move up if the parent node ends later than the + // current node. + if ((!atLastChild || this.openTags.length || this.closeTags.length) && + this.containingNodeEndIndices[depth - 2] > endIndex) { + break + } + + result = true + this.treeCursor.gotoParent() + this.containingNodeTypes.pop() + this.containingNodeChildIndices.pop() + this.containingNodeEndIndices.pop() + --depth + const scopeId = this._currentScopeId() + if (scopeId) this.closeTags.push(scopeId) + } + return result + } + + _moveDown () { + let result = false + const {startIndex} = this.treeCursor + while (this.treeCursor.gotoFirstChild()) { + // Once the iterator has found a scope boundary, it needs to stay at the same + // position, so it should not move down if the first child node starts later than the + // current node. + if ((this.closeTags.length || this.openTags.length) && + this.treeCursor.startIndex > startIndex) { + this.treeCursor.gotoParent() + break + } + + result = true + this.containingNodeTypes.push(this.treeCursor.nodeType) + this.containingNodeChildIndices.push(0) + this.containingNodeEndIndices.push(this.treeCursor.endIndex) + + const scopeId = this._currentScopeId() + if (scopeId) this.openTags.push(scopeId) + } + + return result + } + + _moveRight () { + if (this.treeCursor.gotoNextSibling()) { + const depth = this.containingNodeTypes.length + this.containingNodeTypes[depth - 1] = this.treeCursor.nodeType + this.containingNodeChildIndices[depth - 1]++ + this.containingNodeEndIndices[depth - 1] = this.treeCursor.endIndex + return true + } + } + + _currentScopeId () { + const name = this.languageLayer.grammar.scopeMap.get( this.containingNodeTypes, this.containingNodeChildIndices, this.treeCursor.nodeIsNamed ) - } - - idForScope (scopeName) { - return this.languageLayer.languageMode.grammar.idForScope(scopeName) + if (name) { + return this.languageLayer.languageMode.grammar.idForScope(name) + } } } From ac3bc51c2a07945fb181808d9d031c379946757b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 10:43:05 -0700 Subject: [PATCH 16/28] Improve criteria for when to fold partial vs entire buffer rows --- spec/tree-sitter-language-mode-spec.js | 52 +++++++++++++++++++++++++- src/tree-sitter-language-mode.js | 17 ++++----- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/spec/tree-sitter-language-mode-spec.js b/spec/tree-sitter-language-mode-spec.js index 22d80b823..6d1381a93 100644 --- a/spec/tree-sitter-language-mode-spec.js +++ b/spec/tree-sitter-language-mode-spec.js @@ -722,6 +722,55 @@ describe('TreeSitterLanguageMode', () => { `) }) + it('folds entire buffer rows when necessary to keep words on separate lines', async () => { + const grammar = new TreeSitterGrammar(atom.grammars, jsGrammarPath, { + parser: 'tree-sitter-javascript', + folds: [ + { + start: {type: '{', index: 0}, + end: {type: '}', index: -1} + }, + { + start: {type: '(', index: 0}, + end: {type: ')', index: -1} + } + ] + }) + + buffer.setText(dedent ` + if (a) { + b + } else if (c) { + d + } else { + e + } + `) + + const languageMode = new TreeSitterLanguageMode({buffer, grammar}) + buffer.setLanguageMode(languageMode) + await nextHighlightingUpdate(languageMode) + + // Avoid bringing the `else if...` up onto the same screen line as the preceding `if`. + editor.foldBufferRow(1) + editor.foldBufferRow(3) + expect(getDisplayText(editor)).toBe(dedent ` + if (a) {… + } else if (c) {… + } else { + e + } + `) + + // It's ok to bring the final `}` onto the same screen line as the preceding `else`. + editor.foldBufferRow(5) + expect(getDisplayText(editor)).toBe(dedent ` + if (a) {… + } else if (c) {… + } else {…} + `) + }) + it('can fold nodes of specified types', async () => { const grammar = new TreeSitterGrammar(atom.grammars, jsGrammarPath, { parser: 'tree-sitter-javascript', @@ -1152,7 +1201,8 @@ describe('TreeSitterLanguageMode', () => { expect(getDisplayText(editor)).toBe( `a = html \`
- c\${def(…)}e\${f}g + c\${def(… + )}e\${f}g
\` ` diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index b8a400ce7..70de21021 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -9,6 +9,7 @@ const TextMateLanguageMode = require('./text-mate-language-mode') let nextId = 0 const MAX_RANGE = new Range(Point.ZERO, Point.INFINITY).freeze() const PARSER_POOL = [] +const WORD_REGEX = /\w/ class TreeSitterLanguageMode { static _patchSyntaxNode () { @@ -306,11 +307,13 @@ class TreeSitterLanguageMode { if (!foldEndNode) continue } - if (foldEndNode.endIndex - foldEndNode.startIndex > 1 && foldEndNode.startPosition.row > foldStart.row) { - foldEnd = new Point(foldEndNode.startPosition.row - 1, Infinity) - } else { - foldEnd = foldEndNode.startPosition - if (!pointIsGreater(foldEnd, foldStart)) continue + if (foldEndNode.startPosition.row <= foldStart.row) continue + + foldEnd = foldEndNode.startPosition + if (this.buffer.findInRangeSync( + WORD_REGEX, new Range(foldEnd, new Point(foldEnd.row, Infinity)) + )) { + foldEnd = new Point(foldEnd.row - 1, Infinity) } } else { const {endPosition} = node @@ -1036,10 +1039,6 @@ function compareScopeDescriptorIterators (a, b) { ) } -function pointIsGreater (left, right) { - return left.row > right.row || left.row === right.row && left.column > right.column -} - function last (array) { return array[array.length - 1] } From 806d9311bfc5264720806b64e637009c7a55853e Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Fri, 20 Jul 2018 13:17:56 -0400 Subject: [PATCH 17/28] Introduce atom.project.onDidAddRepository(callback) --- spec/project-spec.js | 35 +++++++++++++++++++++++++++++++++++ src/project.js | 15 +++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/spec/project-spec.js b/spec/project-spec.js index 2d38058fe..a8eedebb2 100644 --- a/spec/project-spec.js +++ b/spec/project-spec.js @@ -969,6 +969,41 @@ describe('Project', () => { }) }) + describe('.onDidAddRepository()', () => { + it('invokes callback when a path is added and the path is the root of a repository', () => { + const observed = [] + const disposable = atom.project.onDidAddRepository((repo) => observed.push(repo)) + + const repositoryPath = path.join(__dirname, '..') + atom.project.addPath(repositoryPath) + expect(observed.length).toBe(1) + expect(observed[0].getOriginURL()).toContain('github.com/atom/atom') + + disposable.dispose() + }) + + it('invokes callback when a path is added and the path is subdirectory of a repository', () => { + const observed = [] + const disposable = atom.project.onDidAddRepository((repo) => observed.push(repo)) + + atom.project.addPath(__dirname) + expect(observed.length).toBe(1) + expect(observed[0].getOriginURL()).toContain('github.com/atom/atom') + + disposable.dispose() + }) + + it('does not invoke callback when a path is added and the path is not part of a repository', () => { + const observed = [] + const disposable = atom.project.onDidAddRepository((repo) => observed.push(repo)) + + atom.project.addPath(temp.mkdirSync('not-a-repository')) + expect(observed.length).toBe(0) + + disposable.dispose() + }) + }) + describe('.relativize(path)', () => { it('returns the path, relative to whichever root directory it is inside of', () => { atom.project.addPath(temp.mkdirSync('another-path')) diff --git a/src/project.js b/src/project.js index 4e51efcf8..d1b10586f 100644 --- a/src/project.js +++ b/src/project.js @@ -234,6 +234,18 @@ class Project extends Model { return this.emitter.on('did-change-files', callback) } + // Public: Invoke the given callback when a repository is added to the + // project. + // + // * `callback` {Function} to be called when a repository is added. + // * `repository` A {GitRepository}. + // + // Returns a {Disposable} on which `.dispose()` can be called to + // unsubscribe. + onDidAddRepository (callback) { + return this.emitter.on('did-add-repository', callback) + } + /* Section: Accessing the git repository */ @@ -400,6 +412,9 @@ class Project extends Model { if (repo) { break } } this.repositories.push(repo != null ? repo : null) + if (repo != null) { + this.emitter.emit('did-add-repository', repo) + } if (options.emitEvent !== false) { this.emitter.emit('did-change-paths', this.getPaths()) From 55da0d8f5d2741c986aff0fa7a39e5584ce09e66 Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Fri, 20 Jul 2018 13:49:18 -0400 Subject: [PATCH 18/28] Introduce atom.project.observeRepositories(callback) --- spec/project-spec.js | 26 ++++++++++++++++++++++++++ src/project.js | 20 ++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/spec/project-spec.js b/spec/project-spec.js index a8eedebb2..b28967eca 100644 --- a/spec/project-spec.js +++ b/spec/project-spec.js @@ -969,6 +969,32 @@ describe('Project', () => { }) }) + describe('.observeRepositories()', () => { + it('invokes the observer with current and future repositories', () => { + const observed = [] + + const directory1 = temp.mkdirSync('git-repo1') + const gitDirPath1 = fs.absolute(path.join(__dirname, 'fixtures', 'git', 'master.git')) + fs.copySync(gitDirPath1, path.join(directory1, '.git')) + + const directory2 = temp.mkdirSync('git-repo2') + const gitDirPath2 = fs.absolute(path.join(__dirname, 'fixtures', 'git', 'repo-with-submodules', 'git.git')) + fs.copySync(gitDirPath2, path.join(directory2, '.git')) + + atom.project.setPaths([directory1]) + + const disposable = atom.project.observeRepositories((repo) => observed.push(repo)) + expect(observed.length).toBe(1) + expect(observed[0].getReferenceTarget('refs/heads/master')).toBe('ef046e9eecaa5255ea5e9817132d4001724d6ae1') + + atom.project.addPath(directory2) + expect(observed.length).toBe(2) + expect(observed[1].getReferenceTarget('refs/heads/master')).toBe('d2b0ad9cbc6f6c4372e8956e5cc5af771b2342e5') + + disposable.dispose() + }) + }) + describe('.onDidAddRepository()', () => { it('invokes callback when a path is added and the path is the root of a repository', () => { const observed = [] diff --git a/src/project.js b/src/project.js index d1b10586f..8ccf60c0b 100644 --- a/src/project.js +++ b/src/project.js @@ -234,6 +234,26 @@ class Project extends Model { return this.emitter.on('did-change-files', callback) } + // Public: Invoke the given callback with all current and future + // repositories in the project. + // + // * `callback` {Function} to be called with current and future + // repositories. + // * `repository` A {GitRepository} that is present at the time of + // subscription or that is added at some later time. + // + // Returns a {Disposable} on which `.dispose()` can be called to + // unsubscribe. + observeRepositories (callback) { + for (const repo of this.repositories) { + if (repo != null) { + callback(repo) + } + } + + return this.onDidAddRepository(callback) + } + // Public: Invoke the given callback when a repository is added to the // project. // From 955544948955e7fbc867206fae5d81cd5cf9f227 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 11:10:12 -0700 Subject: [PATCH 19/28] :art: Clean up TreeSitterLanguageMode --- src/tree-sitter-language-mode.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index 70de21021..f40e2e10b 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -713,8 +713,11 @@ class HighlightIterator { class LayerHighlightIterator { constructor (languageLayer, treeCursor) { this.languageLayer = languageLayer - this.treeCursor = treeCursor + + // The iterator is always positioned at either the start or the end of some node + // in the syntax tree. this.atEnd = false + this.treeCursor = treeCursor // In order to determine which selectors match its current node, the iterator maintains // a list of the current node's ancestors. Because the selectors can use the `:nth-child` @@ -757,7 +760,10 @@ class LayerHighlightIterator { const scopeId = this._currentScopeId() if (scopeId) { if (this.treeCursor.startIndex < targetIndex) { - insertContainingTag(scopeId, this.treeCursor.startIndex, containingTags, containingTagStartIndices) + insertContainingTag( + scopeId, this.treeCursor.startIndex, + containingTags, containingTagStartIndices + ) containingTagEndIndices.push(this.treeCursor.endIndex) } else { this.atEnd = false @@ -785,32 +791,25 @@ class LayerHighlightIterator { } moveToSuccessor () { - let didMove = false this.closeTags.length = 0 this.openTags.length = 0 - while (!(this.done || (didMove && (this.closeTags.length || this.openTags.length)))) { + while (!this.done && !this.closeTags.length && !this.openTags.length) { if (this.atEnd) { if (this._moveRight()) { const scopeId = this._currentScopeId() if (scopeId) this.openTags.push(scopeId) - - didMove = true this.atEnd = false this._moveDown() } else if (this._moveUp(true)) { - didMove = true this.atEnd = true } else { this.done = true } } else if (this._moveDown()) { - didMove = true } else { const scopeId = this._currentScopeId() if (scopeId) this.closeTags.push(scopeId) - - didMove = true this.atEnd = true this._moveUp(false) } From 2a5f7e20c358c6534f4a383feabcc1068cb8c44b Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Fri, 20 Jul 2018 15:12:53 -0400 Subject: [PATCH 20/28] =?UTF-8?q?=E2=9C=85=20Fix=20overly-specific=20asser?= =?UTF-8?q?tion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes failing assertions [1] that unnecessarily assumed that the repository was cloned using `https://github.com/atom/atom.git` as the URL, as opposed to `git@github.com:atom/atom.git` as the URL. [1] https://circleci.com/gh/atom/atom/7981 --- spec/project-spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/project-spec.js b/spec/project-spec.js index b28967eca..e7601253c 100644 --- a/spec/project-spec.js +++ b/spec/project-spec.js @@ -1003,7 +1003,7 @@ describe('Project', () => { const repositoryPath = path.join(__dirname, '..') atom.project.addPath(repositoryPath) expect(observed.length).toBe(1) - expect(observed[0].getOriginURL()).toContain('github.com/atom/atom') + expect(observed[0].getOriginURL()).toContain('atom/atom') disposable.dispose() }) @@ -1014,7 +1014,7 @@ describe('Project', () => { atom.project.addPath(__dirname) expect(observed.length).toBe(1) - expect(observed[0].getOriginURL()).toContain('github.com/atom/atom') + expect(observed[0].getOriginURL()).toContain('atom/atom') disposable.dispose() }) From ea90180f4fbe2626d2710ac4d825dbb72300eae3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 12:38:35 -0700 Subject: [PATCH 21/28] :arrow_up: language-ruby --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 72661dc39..19bc9bd5f 100644 --- a/package.json +++ b/package.json @@ -159,7 +159,7 @@ "language-php": "0.44.0", "language-property-list": "0.9.1", "language-python": "0.51.0", - "language-ruby": "0.72.0", + "language-ruby": "0.72.1", "language-ruby-on-rails": "0.25.3", "language-sass": "0.62.0", "language-shellscript": "0.27.0", From 3e6b95bfbbdda4991aa52f34fc43f59217eda580 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 12:46:12 -0700 Subject: [PATCH 22/28] Preload language-ruby's main module during snapshotting --- src/initialize-application-window.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/src/initialize-application-window.coffee b/src/initialize-application-window.coffee index e3e24eb87..913c83977 100644 --- a/src/initialize-application-window.coffee +++ b/src/initialize-application-window.coffee @@ -38,6 +38,7 @@ if global.isGeneratingSnapshot require('keybinding-resolver') require('language-html') require('language-javascript') + require('language-ruby') require('line-ending-selector') require('link') require('markdown-preview') From 12a3972d908bb74b473496e8fb573c8cd26a6035 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 14:17:29 -0700 Subject: [PATCH 23/28] :art: Remove empty if branch --- src/tree-sitter-language-mode.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tree-sitter-language-mode.js b/src/tree-sitter-language-mode.js index f40e2e10b..bd0b8b1b3 100644 --- a/src/tree-sitter-language-mode.js +++ b/src/tree-sitter-language-mode.js @@ -806,8 +806,7 @@ class LayerHighlightIterator { } else { this.done = true } - } else if (this._moveDown()) { - } else { + } else if (!this._moveDown()) { const scopeId = this._currentScopeId() if (scopeId) this.closeTags.push(scopeId) this.atEnd = true From 24a2d0eda222e24afc81e64b6ee4b72e8c25157f Mon Sep 17 00:00:00 2001 From: Jason Rudolph Date: Fri, 20 Jul 2018 17:53:21 -0400 Subject: [PATCH 24/28] :arrow_up: metrics@1.6.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 19bc9bd5f..995599a09 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,7 @@ "line-ending-selector": "0.7.7", "link": "0.31.4", "markdown-preview": "0.159.20", - "metrics": "1.5.0", + "metrics": "1.6.0", "notifications": "0.70.5", "open-on-github": "1.3.1", "package-generator": "1.3.0", From 3904ff04b8e7a1a064fbe432f060f3d91f2eee99 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 20 Jul 2018 16:28:24 -0700 Subject: [PATCH 25/28] Disable delta nupkg generation for Windows nightly releases --- script/lib/create-windows-installer.js | 1 + 1 file changed, 1 insertion(+) diff --git a/script/lib/create-windows-installer.js b/script/lib/create-windows-installer.js index 447d6aa16..e10d90bbf 100644 --- a/script/lib/create-windows-installer.js +++ b/script/lib/create-windows-installer.js @@ -16,6 +16,7 @@ module.exports = (packagedAppPath) => { loadingGif: path.join(CONFIG.repositoryRootPath, 'resources', 'win', 'loading.gif'), outputDirectory: CONFIG.buildOutputPath, noMsi: true, + noDelta: CONFIG.channel === 'nightly', // Delta packages are broken for nightly versions past nightly9 due to Squirrel/NuGet limitations remoteReleases: `https://atom.io/api/updates${archSuffix}?version=${CONFIG.computedAppVersion}`, setupExe: `AtomSetup${process.arch === 'x64' ? '-x64' : ''}.exe`, setupIcon: path.join(CONFIG.repositoryRootPath, 'resources', 'app-icons', CONFIG.channel, 'atom.ico') From 85dfb15e9764bc87ca735ec05b8aa261c9e8729a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 20 Jul 2018 16:29:01 -0700 Subject: [PATCH 26/28] Use computed version when cleaning up nupkg files in Windows build --- script/lib/create-windows-installer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/lib/create-windows-installer.js b/script/lib/create-windows-installer.js index e10d90bbf..f5e387e7f 100644 --- a/script/lib/create-windows-installer.js +++ b/script/lib/create-windows-installer.js @@ -29,7 +29,7 @@ module.exports = (packagedAppPath) => { } for (let nupkgPath of glob.sync(`${CONFIG.buildOutputPath}/atom-*.nupkg`)) { - if (!nupkgPath.includes(CONFIG.appMetadata.version)) { + if (!nupkgPath.includes(CONFIG.computedAppVersion)) { console.log(`Deleting downloaded nupkg for previous version at ${nupkgPath} to prevent it from being stored as an artifact`) fs.unlinkSync(nupkgPath) } else { From d0a89fb460e00ea73fd6050c4305fb3ec822929a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 20 Jul 2018 17:24:11 -0700 Subject: [PATCH 27/28] :arrow_up: languages, one-syntax themes --- package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 19bc9bd5f..e7addb19d 100644 --- a/package.json +++ b/package.json @@ -87,8 +87,8 @@ "base16-tomorrow-light-theme": "1.5.0", "one-dark-ui": "1.12.3", "one-light-ui": "1.12.3", - "one-dark-syntax": "1.8.3", - "one-light-syntax": "1.8.3", + "one-dark-syntax": "1.8.4", + "one-light-syntax": "1.8.4", "solarized-dark-syntax": "1.1.5", "solarized-light-syntax": "1.1.5", "about": "1.10.0", @@ -149,7 +149,7 @@ "language-html": "0.51.1", "language-hyperlink": "0.16.3", "language-java": "0.30.0", - "language-javascript": "0.129.0", + "language-javascript": "0.129.1", "language-json": "0.19.2", "language-less": "0.34.2", "language-make": "0.22.3", @@ -159,7 +159,7 @@ "language-php": "0.44.0", "language-property-list": "0.9.1", "language-python": "0.51.0", - "language-ruby": "0.72.1", + "language-ruby": "0.72.2", "language-ruby-on-rails": "0.25.3", "language-sass": "0.62.0", "language-shellscript": "0.27.0", From 107ec432e57839e4de12eb5d0034a96c750bf741 Mon Sep 17 00:00:00 2001 From: simurai Date: Mon, 23 Jul 2018 16:39:37 +0900 Subject: [PATCH 28/28] :arrow_up: one-dark/light-ui@v1.12.4 --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index a191a76d7..c951fb3f3 100644 --- a/package.json +++ b/package.json @@ -85,8 +85,8 @@ "atom-light-ui": "0.46.2", "base16-tomorrow-dark-theme": "1.5.0", "base16-tomorrow-light-theme": "1.5.0", - "one-dark-ui": "1.12.3", - "one-light-ui": "1.12.3", + "one-dark-ui": "1.12.4", + "one-light-ui": "1.12.4", "one-dark-syntax": "1.8.4", "one-light-syntax": "1.8.4", "solarized-dark-syntax": "1.1.5",