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.

Sign up for GitHub

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.120 edition #1062

Merged
merged 8 commits into from
Aug 16, 2024
Merged

Tree-sitter rolling fixes, 1.120 edition #1062

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
358cb02
Ignore redundant folds in `getFoldableRanges`
savetheclocktower Jul 18, 2024
ce6989e
Use first significant content on the line (not column 0)…
savetheclocktower Jul 25, 2024
8090558
Bump to the latest `tree-sitter-hyperlink`…
savetheclocktower Jul 25, 2024
af133b0
[language-typescript] Add bitwise operators
savetheclocktower Aug 6, 2024
3b25873
Add some methods for easier customization of language queries…
savetheclocktower Aug 8, 2024
4388bce
Add a spec for the new behavior around dynamic query changing
savetheclocktower Aug 9, 2024
11e2f75
[language-(java|type)script] Fix indentation of `case` statements…
savetheclocktower Aug 9, 2024
6846b9c
Update a few stragglers for the query property renaming
savetheclocktower Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/symbol-provider-tree-sitter/lib/tree-sitter-provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
const CaptureOrganizer = require('./capture-organizer');
const { Emitter } = require('atom');

function layerHasTagsQuery(layer) {
return layer.queries?.tagsQuery ?? layer.tagsQuery;
}
Comment on lines +4 to +6
Copy link
Member

Choose a reason for hiding this comment

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

(re: 6846b9c)

Had to brush up on optional chaining to see that this checks out. It does appear to check out as valid use of optional chaining.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is me being incredibly paranoid, since we'd only fall back to the right side of that ?? expression if someone extracted the newer version of this package and added it to an older version of Pulsar.


class TreeSitterProvider {
constructor() {
this.packageName = 'symbol-provider-tree-sitter';
Expand Down Expand Up @@ -37,7 +41,7 @@ class TreeSitterProvider {
}

// This provider needs at least one layer to have a tags query.
let layers = languageMode.getAllLanguageLayers(l => !!l.tagsQuery);
let layers = languageMode.getAllLanguageLayers(layerHasTagsQuery);
if (layers.length === 0) {
return false;
}
Expand All @@ -64,13 +68,14 @@ class TreeSitterProvider {
// The symbols-view package might've cancelled us in the interim.
if (signal.aborted) return null;

let layers = languageMode.getAllLanguageLayers(l => !!l.tagsQuery);
let layers = languageMode.getAllLanguageLayers(layerHasTagsQuery);
if (layers.length === 0) return null;

for (let layer of layers) {
let extent = layer.getExtent();

let captures = layer.tagsQuery.captures(
let tagsQuery = layer.queries?.tagsQuery ?? layer.tagsQuery;
let captures = tagsQuery.captures(
layer.tree.rootNode,
extent.start,
extent.end
Expand Down
10 changes: 5 additions & 5 deletions src/wasm-tree-sitter-language-mode.js
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

(Regarding ce6989e)

Seems legit! I believe the explanation, anyway! Code doesn't look too crazy, this is mostly adding specs, by diff line count. 👍 Specs are good to have!

Copy link
Member

Choose a reason for hiding this comment

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

(Regarding 358cb02)

Well explained, and I read the associated issue report. Seems like a good way of handling this, effectively the outer fold range starting on a line takes precedence seems to be the practical effect. Reasonable, IMO. 👍

Original file line number Diff line number Diff line change
Expand Up @@ -3290,7 +3290,7 @@ class LanguageLayer {
// propagate errors.
//
// TODO: Warning?
grammar.highlightsQuery = grammar.setQueryForTest(
grammar.setQueryForTest(
'highlightsQuery',
`; (placeholder)`
);
Expand Down Expand Up @@ -3753,8 +3753,8 @@ class LanguageLayer {
}

getLocalReferencesAtPoint(point) {
if (!this.localsQuery) { return []; }
let captures = this.localsQuery.captures(
if (!this.queries.localsQuery) { return []; }
let captures = this.queries.localsQuery.captures(
this.tree.rootNode,
point,
point + 1
Expand All @@ -3777,15 +3777,15 @@ class LanguageLayer {
// EXPERIMENTAL: Given a local reference node, tries to find the node that
// defines it.
findDefinitionForLocalReference(node, captures = null) {
if (!this.localsQuery) { return []; }
if (!this.queries.localsQuery) { return []; }
let name = node.text;
if (!name) { return []; }
let localRange = rangeForNode(node);
let globalScope = this.tree.rootNode;

if (!captures) {
captures = this.groupLocalsCaptures(
this.localsQuery.captures(
this.queries.localsQuery.captures(
globalScope,
globalScope.startPosition,
globalScope.endPosition
Expand Down
Loading