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
47 changes: 47 additions & 0 deletions spec/wasm-tree-sitter-language-mode-spec.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.

(re 4388bce)

Specs are appreciated! 👍

Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,53 @@ describe('WASMTreeSitterLanguageMode', () => {

expect(Array.from(map.values())).toEqual([0, 1, 1, 2, 1, 0]);
});

it('works correctly when straddling an injection boundary, even in the presence of whitespace', async () => {
const jsGrammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

jsGrammar.addInjectionPoint(HTML_TEMPLATE_LITERAL_INJECTION_POINT);

const htmlGrammar = new WASMTreeSitterGrammar(
atom.grammars,
htmlGrammarPath,
htmlConfig
);

htmlGrammar.addInjectionPoint(SCRIPT_TAG_INJECTION_POINT);

atom.grammars.addGrammar(jsGrammar);
atom.grammars.addGrammar(htmlGrammar);

// This is just like the test above, except that we're indented a bit.
// Now the edge of the injection isn't at the beginning of the line; it's
// at the beginning of the first _text_ on the line.
buffer.setText(dedent`
<html>
<head>
<script>
let foo;
if (foo) {
debug(true);
}
</script>
</head>
</html>
`);

const languageMode = new WASMTreeSitterLanguageMode({
grammar: htmlGrammar,
buffer,
config: atom.config,
grammars: atom.grammars
});

buffer.setLanguageMode(languageMode);
await languageMode.ready;

let map = languageMode.suggestedIndentForBufferRows(0, 9, editor.getTabLength());

expect(Array.from(map.values())).toEqual([0, 1, 2, 3, 3, 4, 3, 2, 1, 0]);
})
});

describe('folding', () => {
Expand Down
13 changes: 11 additions & 2 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 @@ -1605,6 +1605,8 @@ class WASMTreeSitterLanguageMode {
indentDelta -= clamp(dedentNextDelta, 0, 1);

let dedentDelta = 0;
let line = this.buffer.lineForRow(row);
let rowStartingColumn = Math.max(line.search(/\S/), 0);

if (!options.skipDedentCheck) {
scopeResolver.reset();
Expand All @@ -1613,7 +1615,11 @@ class WASMTreeSitterLanguageMode {
// starting indent is on the current line. But it might not extend to the
// current line, so we should determine which layer is in charge of the
// second phase.
let rowStart = new Point(row, 0);
//
// The comparison point we use is that of the first character on the
// line. If we start earlier than that, we might not pick up on the
// presence of an injection layer.
let rowStart = new Point(row, rowStartingColumn);
let dedentControllingLayer = this.controllingLayerAtPoint(
rowStart,
(layer) => {
Expand Down Expand Up @@ -1885,8 +1891,11 @@ class WASMTreeSitterLanguageMode {
// By the time this function runs, we probably know enough to be sure of
// which layer controls the beginning of this row, even if we don't know
// which one owns the position at the cursor.
//
// Use the position of the first text on the line as the reference point.
let rowStartingColumn = Math.max(line.search(/\S/), 0);
let controllingLayer = this.controllingLayerAtPoint(
new Point(row, 0),
new Point(row, rowStartingColumn),
(layer) => !!layer.indentsQuery
);

Expand Down
Loading