Skip to content

Commit

Permalink
Fix range end position in piece tree buffer (#238658)
Browse files Browse the repository at this point in the history 
* Fix range end position in piece tree buffer
also fix refresh ranges in token guess

* Simplify first tree sitter tokens

* Compute newRange end for changes in tree sitter

* Fix tests
  • Loading branch information
@alexr00
alexr00 authored Jan 24, 2025
1 parent cd9657a commit 340c573
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 82 deletions.
18 changes: 3 additions & 15 deletions src/vs/editor/common/model/pieceTreeTextBuffer/pieceTreeTextBuffer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ export interface IValidatedEditOperation {
isAutoWhitespaceEdit: boolean;
}

export interface IValidatedEditOperationWithReverseRange extends IValidatedEditOperation {
reverseRange: Range;
}

interface IReverseSingleEditOperation extends IValidEditOperation {
sortIndex: number;
}
Expand Down Expand Up @@ -327,7 +323,7 @@ export class PieceTreeTextBuffer extends Disposable implements ITextBuffer {
}

// Delta encode operations
const reverseRanges = PieceTreeTextBuffer._getInverseEditRanges(operations);
const reverseRanges = (computeUndoEdits || recordTrimAutoWhitespace ? PieceTreeTextBuffer._getInverseEditRanges(operations) : []);
const newTrimAutoWhitespaceCandidates: { lineNumber: number; oldContent: string }[] = [];
if (recordTrimAutoWhitespace) {
for (let i = 0; i < operations.length; i++) {
Expand Down Expand Up @@ -377,19 +373,12 @@ export class PieceTreeTextBuffer extends Disposable implements ITextBuffer {
}
}

const operationsWithReverseRanges: IValidatedEditOperationWithReverseRange[] = new Array(operations.length);
for (let i = 0; i < operations.length; i++) {
operationsWithReverseRanges[i] = {
...operations[i],
reverseRange: reverseRanges[i]
};
}

this._mightContainRTL = mightContainRTL;
this._mightContainUnusualLineTerminators = mightContainUnusualLineTerminators;
this._mightContainNonBasicASCII = mightContainNonBasicASCII;

const contentChanges = this._doApplyEdits(operationsWithReverseRanges);
const contentChanges = this._doApplyEdits(operations);

let trimAutoWhitespaceLineNumbers: number[] | null = null;
if (recordTrimAutoWhitespace && newTrimAutoWhitespaceCandidates.length > 0) {
Expand Down Expand Up @@ -487,7 +476,7 @@ export class PieceTreeTextBuffer extends Disposable implements ITextBuffer {
};
}

private _doApplyEdits(operations: IValidatedEditOperationWithReverseRange[]): IInternalModelContentChange[] {
private _doApplyEdits(operations: IValidatedEditOperation[]): IInternalModelContentChange[] {
operations.sort(PieceTreeTextBuffer._sortOpsDescending);

const contentChanges: IInternalModelContentChange[] = [];
Expand Down Expand Up @@ -520,7 +509,6 @@ export class PieceTreeTextBuffer extends Disposable implements ITextBuffer {
contentChanges.push({
range: contentChangeRange,
rangeLength: op.rangeLength,
rangeEndPosition: op.reverseRange.getEndPosition(),
text: op.text,
rangeOffset: op.rangeOffset,
forceMoveMarkers: op.forceMoveMarkers
Expand Down
1 change: 0 additions & 1 deletion src/vs/editor/common/model/textModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ export class TextModel extends Disposable implements model.ITextModel, IDecorati
range: range,
rangeOffset: rangeOffset,
rangeLength: rangeLength,
rangeEndPosition: rangeEndPosition,
text: text,
}],
eol: this._buffer.getEOL(),
Expand Down
22 changes: 11 additions & 11 deletions src/vs/editor/common/model/tokenStore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,15 @@ export class TokenStore implements IDisposable {
this._root = this.createFromUpdates(tokens, true);
}

private createFromUpdates(tokens: TokenUpdate[], isNew?: boolean): Node {
private createFromUpdates(tokens: TokenUpdate[], needsRefresh?: boolean): Node {
let newRoot: Node = {
length: tokens[0].length,
token: tokens[0].token,
height: 0,
needsRefresh: isNew
needsRefresh
};
for (let j = 1; j < tokens.length; j++) {
newRoot = append(newRoot, { length: tokens[j].length, token: tokens[j].token, height: 0, needsRefresh: isNew });
newRoot = append(newRoot, { length: tokens[j].length, token: tokens[j].token, height: 0, needsRefresh });
}
return newRoot;
}
Expand All @@ -227,22 +227,22 @@ export class TokenStore implements IDisposable {
*
* @param tokens tokens are in sequence in the document.
*/
update(length: number, tokens: TokenUpdate[]) {
update(length: number, tokens: TokenUpdate[], needsRefresh?: boolean) {
if (tokens.length === 0) {
return;
}
this.replace(length, tokens[0].startOffsetInclusive, tokens);
this.replace(length, tokens[0].startOffsetInclusive, tokens, needsRefresh);
}

delete(length: number, startOffset: number) {
this.replace(length, startOffset, []);
this.replace(length, startOffset, [], true);
}

/**
*
* @param tokens tokens are in sequence in the document.
*/
private replace(length: number, updateOffsetStart: number, tokens: TokenUpdate[]) {
private replace(length: number, updateOffsetStart: number, tokens: TokenUpdate[], needsRefresh?: boolean) {
const firstUnchangedOffsetAfterUpdate = updateOffsetStart + length;
// Find the last unchanged node preceding the update
const precedingNodes: Node[] = [];
Expand All @@ -260,7 +260,7 @@ export class TokenStore implements IDisposable {
continue;
} else if (isLeaf(node.node) && (currentOffset < updateOffsetStart)) {
// We have a partial preceding node
precedingNodes.push({ length: updateOffsetStart - currentOffset, token: node.node.token, height: 0 });
precedingNodes.push({ length: updateOffsetStart - currentOffset, token: node.node.token, height: 0, needsRefresh: needsRefresh || node.node.needsRefresh });
// Node could also be postceeding, so don't continue
}

Expand All @@ -274,7 +274,7 @@ export class TokenStore implements IDisposable {
continue;
} else if (isLeaf(node.node) && (currentOffset + node.node.length >= firstUnchangedOffsetAfterUpdate)) {
// we have a partial postceeding node
postcedingNodes.push({ length: currentOffset + node.node.length - firstUnchangedOffsetAfterUpdate, token: node.node.token, height: 0 });
postcedingNodes.push({ length: currentOffset + node.node.length - firstUnchangedOffsetAfterUpdate, token: node.node.token, height: 0, needsRefresh: needsRefresh || node.node.needsRefresh });
continue;
}

Expand All @@ -290,7 +290,7 @@ export class TokenStore implements IDisposable {

let allNodes: Node[];
if (tokens.length > 0) {
allNodes = precedingNodes.concat(this.createFromUpdates(tokens), postcedingNodes);
allNodes = precedingNodes.concat(this.createFromUpdates(tokens, needsRefresh), postcedingNodes);
} else {
allNodes = precedingNodes.concat(postcedingNodes);
}
Expand Down Expand Up @@ -445,7 +445,7 @@ export class TokenStore implements IDisposable {
const indent = ' '.repeat(depth);

if (isLeaf(node)) {
result.push(`${indent}Leaf(length: ${node.length}, token: ${node.token})\n`);
result.push(`${indent}Leaf(length: ${node.length}, token: ${node.token}, refresh: ${node.needsRefresh})\n`);
} else {
result.push(`${indent}List(length: ${node.length})\n`);
// Push children in reverse order so they get processed left-to-right
Expand Down
9 changes: 5 additions & 4 deletions src/vs/editor/common/model/treeSitterTokenStoreService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,18 @@ class TreeSitterTokenizationStoreService implements ITreeSitterTokenizationStore

storeInfo.guessVersion = e.versionId;
for (const change of e.changes) {
storeInfo.store.markForRefresh(change.rangeOffset, change.rangeOffset + change.rangeLength);
if (change.text.length > change.rangeLength) {
const oldToken = storeInfo.store.getTokenAt(change.rangeOffset)!;
// Insert. Just grow the token at this position to include the insert.
const newToken = { startOffsetInclusive: oldToken.startOffsetInclusive, length: oldToken.length + change.text.length - change.rangeLength, token: oldToken.token };
storeInfo.store.update(oldToken.length, [newToken]);
storeInfo.store.update(oldToken.length, [newToken], true);
} else if (change.text.length < change.rangeLength) {
// Delete. Delete the tokens at the corresponding range.
const deletedCharCount = change.rangeLength - change.text.length;
storeInfo.store.delete(deletedCharCount, change.rangeOffset);
}
const refreshLength = change.rangeLength > change.text.length ? change.rangeLength : change.text.length;
storeInfo.store.markForRefresh(change.rangeOffset, change.rangeOffset + refreshLength);
}
}));
disposables.add(model.onWillDispose(() => {
Expand Down Expand Up @@ -105,8 +106,8 @@ class TreeSitterTokenizationStoreService implements ITreeSitterTokenizationStore

existingTokens.accurateVersion = version;
for (const update of updates) {
const lastToken = update.newTokens[update.newTokens.length - 1];
const oldRangeLength = (existingTokens.guessVersion >= version) ? (lastToken.startOffsetInclusive + lastToken.length - update.newTokens[0].startOffsetInclusive) : update.oldRangeLength;
const lastToken = update.newTokens.length > 0 ? update.newTokens[update.newTokens.length - 1] : undefined;
const oldRangeLength = ((existingTokens.guessVersion >= version) && lastToken) ? (lastToken.startOffsetInclusive + lastToken.length - update.newTokens[0].startOffsetInclusive) : update.oldRangeLength;
existingTokens.store.update(oldRangeLength, update.newTokens);
}
}
Expand Down
20 changes: 9 additions & 11 deletions src/vs/editor/common/services/treeSitter/treeSitterParserService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { PromiseResult } from '../../../../base/common/observable.js';
import { Range } from '../../core/range.js';
import { Position } from '../../core/position.js';
import { LimitedQueue } from '../../../../base/common/async.js';
import { TextLength } from '../../core/textLength.js';

const EDITOR_TREESITTER_TELEMETRY = 'editor.experimental.treeSitterTelemetry';
const MODULE_LOCATION_SUBPATH = `@vscode/tree-sitter-wasm/wasm`;
Expand Down Expand Up @@ -350,22 +351,19 @@ export class TreeSitterParseResult implements IDisposable, ITreeSitterParseResul

private _applyEdits(changes: IModelContentChange[], version: number) {
for (const change of changes) {
this._tree?.edit({
const originalTextLength = TextLength.ofRange(Range.lift(change.range));
const newTextLength = TextLength.ofText(change.text);
const summedTextLengths = change.text.length === 0 ? newTextLength : originalTextLength.add(newTextLength);
const edit = {
startIndex: change.rangeOffset,
oldEndIndex: change.rangeOffset + change.rangeLength,
newEndIndex: change.rangeOffset + change.text.length,
startPosition: { row: change.range.startLineNumber - 1, column: change.range.startColumn - 1 },
oldEndPosition: { row: change.range.endLineNumber - 1, column: change.range.endColumn - 1 },
newEndPosition: { row: change.rangeEndPosition.lineNumber - 1, column: change.rangeEndPosition.column - 1 }
});
this._lastFullyParsedWithEdits?.edit({
startIndex: change.rangeOffset,
oldEndIndex: change.rangeOffset + change.rangeLength,
newEndIndex: change.rangeOffset + change.text.length,
startPosition: { row: change.range.startLineNumber - 1, column: change.range.startColumn - 1 },
oldEndPosition: { row: change.range.endLineNumber - 1, column: change.range.endColumn - 1 },
newEndPosition: { row: change.rangeEndPosition.lineNumber - 1, column: change.rangeEndPosition.column - 1 }
});
newEndPosition: { row: change.range.startLineNumber + summedTextLengths.lineCount - 1, column: summedTextLengths.columnCount }
};
this._tree?.edit(edit);
this._lastFullyParsedWithEdits?.edit(edit);
}
this._editVersion = version;
}
Expand Down
5 changes: 0 additions & 5 deletions src/vs/editor/common/textModelEvents.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Position } from './core/position.js';
import { IRange } from './core/range.js';
import { Selection } from './core/selection.js';
import { IModelDecoration, InjectedTextOptions } from './model.js';
Expand Down Expand Up @@ -46,10 +45,6 @@ export interface IModelContentChange {
* The length of the range that got replaced.
*/
readonly rangeLength: number;
/**
* The new end position of the range that got replaced.
*/
readonly rangeEndPosition: Position;
/**
* The new text for the range.
*/
Expand Down
16 changes: 8 additions & 8 deletions src/vs/editor/test/browser/widget/observableCodeEditor.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ suite("CodeEditorWidget", () => {

assert.deepStrictEqual(log.getAndClearEntries(), [
'handle change: editor.onDidType "abc"',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":2},"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,2 -> 1,2]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":3},"text":"b","rangeOffset":1}],"eol":"\\n","versionId":3}',
'handle change: editor.versionId {"changes":[{"range":"[1,3 -> 1,3]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":4},"text":"c","rangeOffset":2}],"eol":"\\n","versionId":4}',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,2 -> 1,2]","rangeLength":0,"text":"b","rangeOffset":1}],"eol":"\\n","versionId":3}',
'handle change: editor.versionId {"changes":[{"range":"[1,3 -> 1,3]","rangeLength":0,"text":"c","rangeOffset":2}],"eol":"\\n","versionId":4}',
'handle change: editor.selections {"selection":"[1,4 -> 1,4]","modelVersionId":4,"oldSelections":["[1,1 -> 1,1]"],"oldModelVersionId":1,"source":"keyboard","reason":0}',
'running derived: selection: [1,4 -> 1,4], value: 4',
]);
Expand All @@ -96,9 +96,9 @@ suite("CodeEditorWidget", () => {

assert.deepStrictEqual(log.getAndClearEntries(), [
'handle change: editor.onDidType "abc"',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":2},"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,2 -> 1,2]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":3},"text":"b","rangeOffset":1}],"eol":"\\n","versionId":3}',
'handle change: editor.versionId {"changes":[{"range":"[1,3 -> 1,3]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":4},"text":"c","rangeOffset":2}],"eol":"\\n","versionId":4}',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,2 -> 1,2]","rangeLength":0,"text":"b","rangeOffset":1}],"eol":"\\n","versionId":3}',
'handle change: editor.versionId {"changes":[{"range":"[1,3 -> 1,3]","rangeLength":0,"text":"c","rangeOffset":2}],"eol":"\\n","versionId":4}',
'handle change: editor.selections {"selection":"[1,4 -> 1,4]","modelVersionId":4,"oldSelections":["[1,1 -> 1,1]"],"oldModelVersionId":1,"source":"keyboard","reason":0}',
'running derived: selection: [1,4 -> 1,4], value: 4',
]);
Expand Down Expand Up @@ -134,7 +134,7 @@ suite("CodeEditorWidget", () => {
">>> before get",
"<<< after get",
'handle change: editor.onDidType "a"',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":2},"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.selections {"selection":"[1,2 -> 1,2]","modelVersionId":2,"oldSelections":["[1,1 -> 1,1]"],"oldModelVersionId":1,"source":"keyboard","reason":0}',
"running derived: selection: [1,2 -> 1,2], value: 2",
]);
Expand Down Expand Up @@ -172,7 +172,7 @@ suite("CodeEditorWidget", () => {
"running derived: selection: [1,2 -> 1,2], value: 2",
"<<< after get",
'handle change: editor.onDidType "a"',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"rangeEndPosition":{"lineNumber":1,"column":2},"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.versionId {"changes":[{"range":"[1,1 -> 1,1]","rangeLength":0,"text":"a","rangeOffset":0}],"eol":"\\n","versionId":2}',
'handle change: editor.selections {"selection":"[1,2 -> 1,2]","modelVersionId":2,"oldSelections":["[1,1 -> 1,1]"],"oldModelVersionId":1,"source":"keyboard","reason":0}',
"running derived: selection: [1,2 -> 1,2], value: 2",
]);
Expand Down
4 changes: 0 additions & 4 deletions src/vs/monaco.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2914,10 +2914,6 @@ declare namespace monaco.editor {
* The length of the range that got replaced.
*/
readonly rangeLength: number;
/**
* The new end position of the range that got replaced.
*/
readonly rangeEndPosition: Position;
/**
* The new text for the range.
*/
Expand Down
6 changes: 0 additions & 6 deletions src/vs/workbench/api/test/browser/extHostDocumentData.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ suite('ExtHostDocumentData', () => {
range: { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 1 },
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: '\t '
}],
eol: undefined!,
Expand Down Expand Up @@ -164,7 +163,6 @@ suite('ExtHostDocumentData', () => {
range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: ''
}],
eol: undefined!,
Expand All @@ -185,7 +183,6 @@ suite('ExtHostDocumentData', () => {
range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: 'is could be'
}],
eol: undefined!,
Expand All @@ -206,7 +203,6 @@ suite('ExtHostDocumentData', () => {
range: { startLineNumber: 1, startColumn: 3, endLineNumber: 1, endColumn: 6 },
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: 'is could be\na line with number'
}],
eol: undefined!,
Expand All @@ -230,7 +226,6 @@ suite('ExtHostDocumentData', () => {
range: { startLineNumber: 1, startColumn: 3, endLineNumber: 2, endColumn: 6 },
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: ''
}],
eol: undefined!,
Expand Down Expand Up @@ -421,7 +416,6 @@ suite('ExtHostDocumentData updates line mapping', () => {
range: range,
rangeOffset: undefined!,
rangeLength: undefined!,
rangeEndPosition: undefined!,
text: text
}],
eol: eol!,
Expand Down
2 changes: 0 additions & 2 deletions src/vs/workbench/api/test/browser/extHostDocumentSaveParticipant.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ suite('ExtHostDocumentSaveParticipant', () => {
documents.$acceptModelChanged(resource, {
changes: [{
range: { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 1 },
rangeEndPosition: undefined!,
rangeOffset: undefined!,
rangeLength: undefined!,
text: 'bar'
Expand Down Expand Up @@ -330,7 +329,6 @@ suite('ExtHostDocumentSaveParticipant', () => {
changes: [{
range,
text,
rangeEndPosition: undefined!,
rangeOffset: undefined!,
rangeLength: undefined!,
}],
Expand Down
21 changes: 6 additions & 15 deletions src/vs/workbench/services/treeSitter/browser/treeSitterTokenizationFeature.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,26 +136,17 @@ export class TreeSitterTokenizationSupport extends Disposable implements ITreeSi
}));
}

private _createEmptyTokens(captures: { tree: ITreeSitterParseResult | undefined; captures: QueryCapture[] }, modelEndOffset: number) {
private _createEmptyTokens(textModel: ITextModel) {
const languageId = this._languageIdCodec.encodeLanguageId(this._languageId);
const emptyToken = this._emptyToken(languageId);
const capturedTokens = this._createTokensFromCaptures(captures.tree, captures.captures, 0, modelEndOffset);
if (!capturedTokens) {
return;
}
const emptyTokens: EndOffsetToken[] = capturedTokens.endOffsets.map(capture => ({ endOffset: capture.endOffset, metadata: emptyToken }));
return this._rangeTokensAsUpdates(0, emptyTokens);
const modelEndOffset = textModel.getValueLength();

const emptyTokens: TokenUpdate[] = [{ token: emptyToken, length: modelEndOffset, startOffsetInclusive: 0 }];
return emptyTokens;
}

private _firstTreeUpdate(textModel: ITextModel, versionId: number, ranges: { readonly fromLineNumber: number; readonly toLineNumber: number }[]) {
const modelEndOffset = textModel.getValueLength();
const editorEndPosition = textModel.getPositionAt(modelEndOffset);
const captures = this._getTreeAndCaptures(new Range(1, 1, editorEndPosition.lineNumber, editorEndPosition.column), textModel);
if (captures.captures.length === 0) {
return;
}
// Make empty tokens to populate the store
const tokens: TokenUpdate[] = this._createEmptyTokens(captures, modelEndOffset) ?? [];
const tokens: TokenUpdate[] = this._createEmptyTokens(textModel);
this._tokenizationStoreService.setTokens(textModel, tokens);
this._setViewPortTokens(textModel, versionId);
}
Expand Down

0 comments on commit 340c573

Please sign in to comment.