Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree-sitter rolling fixes, 1.127 edition #1240

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

savetheclocktower
Copy link
Contributor

First commit is a monumental one: Upgrade to web-tree-sitter v0.25.3.

This is a big upgrade! web-tree-sitter has been rewritten in TypeScript and has made some backwards-incompatible changes to its exports — but the changes make sense, so it's OK.

Tree-sitter also bumped its ABI, so we had to keep up or else risk being unable to support parsers generated with newer tree-sitter-cli versions.

I've also updated the instructions for generating the web-tree-sitter bundle, since the process has changed again.

@@ -2,7 +2,7 @@ const fs = require('fs');
const path = require('path');
const Grim = require('grim');
const dedent = require('dedent');
const Parser = require('./web-tree-sitter');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

web-tree-sitter used to have a single export — the Parser object — and hang a small number of other classes off of it (like Parser.Language). This made no sense. Now Language (and many other classes) are available to import directly.

@@ -299,7 +299,7 @@ module.exports = class WASMTreeSitterGrammar {
if (!language) { return null; }
let query = this.queryCache.get(queryType);
if (!query) {
query = language.query(this[queryType]);
query = new Query(language, this[queryType]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query instantiation has moved to a constructor. The old Language::query method would still have worked, but it's deprecated, so we might as well convert it now and avoid warnings in the console.

@@ -1,4 +1,4 @@
const Parser = require('./web-tree-sitter');
const { Node, Parser } = require('./web-tree-sitter');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We add a property and a method to Node for backward compatibility. We previously were unable to get a reference to Node.prototype until the first time we had an instance of a Node, so we had to apply the patch awkwardly at an arbitrary point in the parsing lifecycle.

Now that we can import Node directly, our lives are easier; we can do the patch immediately after import.

Comment on lines +87 to +115
## TEMPORARY: Fix incompatible syntax

Technically, the new build process for `web-tree-sitter` generates some syntax that’s incompatible with Pulsar running on Electron 12/Node 14. [Static initialization blocks](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Static_initialization_blocks) are a syntactic enhancement that doesn’t arrive until Node v16.11.0, but they’re easy to convert to a more broadly compatible syntax.

Find a block that looks like this:

```js
// src/lookahead_iterator.ts
var LookaheadIterator = class {
static {
__name(this, "LookaheadIterator");
}
// …
};
```

It’s lucky for us that the entire `static {` block can simply be removed without anything breaking; it’s mainly for friendlier stack traces. But we can go further and simply move it below the class declaration:

```js
// src/lookahead_iterator.ts
var LookaheadIterator = class {
// …
};
__name(LookaheadIterator, "LookaheadIterator");
```

This needs to be done in about a half-dozen places.

Once we upgrade to a more recent version of Electron, this step won’t be necessary.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd been beta-testing the rewritten web-tree-sitter so I could confirm for @amaanq that it still worked in Pulsar, since we're probably the highest-profile consumers of web-tree-sitter right now. Lazily and foolishly, I only tested in PulsarNext — or else I'd have caught this syntactic addition that Pulsar-on-Electron-12 does not support.

We're very lucky that it appears to be the only thing that's incompatible with Node 14, and that it's ultimately very easy to rewrite. Once we actually upgrade our version of Electron, we can skip this part.

This is a big upgrade! `web-tree-sitter` has been rewritten in TypeScript and has made some backwards-incompatible changes to its exports — all of which are good things.

I've also updated the instructions for generating the `web-tree-sitter` bundle.
* Make function parameters spanning multiple lines fold properly
* Make `try`/`except` clauses fold properly, with `try` folding only up to the first `except`
* Add `folding-spec.js` to test these cases and others and guard them against regressions

Also fix a silly mistake in an indentation query.

Fixes #1216.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant