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

Merged
merged 8 commits into from
Aug 16, 2024
95 changes: 95 additions & 0 deletions spec/wasm-tree-sitter-language-mode-spec.js
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 @@ -2208,6 +2208,101 @@ describe('WASMTreeSitterLanguageMode', () => {
`);
});

it('does not enumerate redundant folds', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

await grammar.setQueryForTest('foldsQuery', `
(statement_block) @fold
(object) @fold
`);

// This odd way of formatting code produces a scenario where two folds
// would start on the same line. The second of the two folds would never
// be seen when toggling the fold on that line, so we shouldn't treat it
// as a valid fold for any other purpose.
buffer.setText(dedent`
if (foo) {results.push({
bar: 'baz'
})}
`);

const languageMode = new WASMTreeSitterLanguageMode({ grammar, buffer });
buffer.setLanguageMode(languageMode);
await languageMode.ready;

let ranges = languageMode.getFoldableRanges();
expect(ranges.length).toBe(1);
})

it('is not flummoxed by redundant folds when performing foldAllAtIndentLevel', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig);

await grammar.setQueryForTest('foldsQuery', `
(statement_block) @fold
(object) @fold
`);

buffer.setText(dedent`
function foo() {
if (true) {
if (foo) {results.push({
bar: 'baz'
})}
}
}

function bar() {
if (false) {
// TODO
}
}
`);

const languageMode = new WASMTreeSitterLanguageMode({ grammar, buffer });
buffer.setLanguageMode(languageMode);
await languageMode.ready;

editor.foldAllAtIndentLevel(1);
expect(getDisplayText(editor)).toBe(dedent`
function foo() {
if (true) {…}
}

function bar() {
if (false) {…}
}
`);

buffer.setText(dedent`
function foo() {
if (true) {
if (foo) {
results.push({
bar: 'baz'
})}
}
}

function bar() {
if (false) {
// TODO
}
}
`);
await languageMode.atTransactionEnd();

editor.foldAllAtIndentLevel(1);
expect(getDisplayText(editor)).toBe(dedent`
function foo() {
if (true) {…}
}

function bar() {
if (false) {…}
}
`);
})

it('can target named vs anonymous nodes as fold boundaries', async () => {
const grammar = new WASMTreeSitterGrammar(atom.grammars, rubyGrammarPath, rubyConfig);

Expand Down
35 changes: 30 additions & 5 deletions src/wasm-tree-sitter-language-mode.js
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 @@ -1005,7 +1005,6 @@ class WASMTreeSitterLanguageMode {
let layers = this.getAllLanguageLayers();
for (let layer of layers) {
let folds = layer.foldResolver.getAllFoldRanges();

for (let fold of folds) {
rangeTree = rangeTree.insert(fold.start, { start: fold });
rangeTree = rangeTree.insert(fold.end, { end: fold });
Expand Down Expand Up @@ -2284,8 +2283,14 @@ class FoldResolver {
let end = Point.fromObject({ row: row + 1, column: 0 });

let tree = this.layer.getOrParseTree({ force: false });
// Search for folds that begin somewhere on the given row.
let iterator = this.getOrCreateBoundariesIterator(tree.rootNode, start, end);

// More than one fold can match for a given row, so we'll stop as soon as
// we find the fold that starts earliest on the row. (The fold itself will
// be “resolved” in such a way that it doesn't begin until the end of the
// row, but we still consider the intrinsic range of the fold capture when
// deciding which one to honor.)
while (iterator.key) {
if (comparePoints(iterator.key.position, end) >= 0) { break; }
let capture = iterator.value;
Expand All @@ -2308,22 +2313,42 @@ class FoldResolver {
}

// Returns all valid fold ranges in this language layer.
//
// There are two rules about folds that we can't change:
//
// 1. A fold must collapse at least one line’s worth of content.
// 2. The UI for expanding and collapsing folds envisions that each line can
// manage a maximum of _one_ fold.
//
// Hence a fold range is “valid” when it
// * resolves to a range that spans more than one line;
// * starts on a line that hasn't already been promised to an earlier fold.
getAllFoldRanges() {
if (!this.layer.tree || !this.layer.foldsQuery) { return []; }
let range = this.layer.getExtent();
// We use a Tree-sitter query to find folds; then we arrange the folds in
// buffer order. The first valid fold we find on a given line is included
// in the list; any other folds on the line are ignored.
let iterator = this.getOrCreateBoundariesIterator(
this.layer.tree.rootNode, range.start, range.end);

let results = [];
let lastValidFoldRange = null;
while (iterator.key) {
let capture = iterator.value;
let { name } = capture;
let range;
if (name === 'fold') {
let range = this.resolveRangeForSimpleFold(capture);
if (this.isValidFold(range)) { results.push(range); }
range = this.resolveRangeForSimpleFold(capture);
} else if (name === 'fold.start') {
let range = this.resolveRangeForDividedFold(capture);
if (this.isValidFold(range)) { results.push(range); }
range = this.resolveRangeForDividedFold(capture);
}
if (this.isValidFold(range)) {
// Recognize only the first fold for each row.
if (lastValidFoldRange?.start?.row !== range.start.row) {
results.push(range);
lastValidFoldRange = range;
}
}
iterator.next();
}
Expand Down
Loading