Skip to content

Commit 5d85d2e

Browse files
jriekenjrieken
committed
suggest - don't send default ranges for each item
1 parent 1a5ffab commit 5d85d2e

3 files changed

Lines changed: 64 additions & 50 deletions

File tree

‎src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as search from 'vs/workbench/contrib/search/common/search';
1111
import { CancellationToken } from 'vs/base/common/cancellation';
1212
import { Position as EditorPosition } from 'vs/editor/common/core/position';
1313
import { Range as EditorRange, IRange } from 'vs/editor/common/core/range';
14-
import { ExtHostContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, MainContext, IExtHostContext, ILanguageConfigurationDto, IRegExpDto, IIndentationRuleDto, IOnEnterRuleDto, ILocationDto, IWorkspaceSymbolDto, reviveWorkspaceEditDto, IDocumentFilterDto, IDefinitionLinkDto, ISignatureHelpProviderMetadataDto, ILinkDto, ICallHierarchyItemDto, ISuggestDataDto, ICodeActionDto, ISuggestDataDtoField } from '../common/extHost.protocol';
14+
import { ExtHostContext, MainThreadLanguageFeaturesShape, ExtHostLanguageFeaturesShape, MainContext, IExtHostContext, ILanguageConfigurationDto, IRegExpDto, IIndentationRuleDto, IOnEnterRuleDto, ILocationDto, IWorkspaceSymbolDto, reviveWorkspaceEditDto, IDocumentFilterDto, IDefinitionLinkDto, ISignatureHelpProviderMetadataDto, ILinkDto, ICallHierarchyItemDto, ISuggestDataDto, ICodeActionDto, ISuggestDataDtoField, ISuggestResultDtoField } from '../common/extHost.protocol';
1515
import { LanguageConfigurationRegistry } from 'vs/editor/common/modes/languageConfigurationRegistry';
1616
import { LanguageConfiguration, IndentationRule, OnEnterRule } from 'vs/editor/common/modes/languageConfiguration';
1717
import { IModeService } from 'vs/editor/common/services/modeService';
@@ -368,10 +368,15 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha
368368
if (!result) {
369369
return result;
370370
}
371+
371372
return {
372-
suggestions: result.b.map(d => MainThreadLanguageFeatures._inflateSuggestDto(result.a, d)),
373-
incomplete: result.c,
374-
dispose: () => typeof result.x === 'number' && this._proxy.$releaseCompletionItems(handle, result.x)
373+
suggestions: result[ISuggestResultDtoField.completions].map(d => MainThreadLanguageFeatures._inflateSuggestDto(result[ISuggestResultDtoField.defaultRanges], d)),
374+
incomplete: result[ISuggestResultDtoField.isIncomplete] || false,
375+
dispose: () => {
376+
if (typeof result.x === 'number') {
377+
this._proxy.$releaseCompletionItems(handle, result.x);
378+
}
379+
}
375380
};
376381
});
377382
}

‎src/vs/workbench/api/common/extHost.protocol.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,11 +1030,17 @@ export interface ISuggestDataDto {
10301030
x?: ChainedCacheId;
10311031
}
10321032

1033+
export const enum ISuggestResultDtoField {
1034+
defaultRanges = 'a',
1035+
completions = 'b',
1036+
isIncomplete = 'c'
1037+
}
1038+
10331039
export interface ISuggestResultDto {
1040+
[ISuggestResultDtoField.defaultRanges]: { insert: IRange, replace: IRange; };
1041+
[ISuggestResultDtoField.completions]: ISuggestDataDto[];
1042+
[ISuggestResultDtoField.isIncomplete]: undefined | true;
10341043
x?: number;
1035-
a: { insert: IRange, replace: IRange; };
1036-
b: ISuggestDataDto[];
1037-
c?: true;
10381044
}
10391045

10401046
export interface ISignatureHelpDto {

‎src/vs/workbench/api/common/extHostLanguageFeatures.ts‎

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -819,19 +819,20 @@ class SuggestAdapter {
819819
const disposables = new DisposableStore();
820820
this._disposables.set(pid, disposables);
821821

822+
const completions: extHostProtocol.ISuggestDataDto[] = [];
822823
const result: extHostProtocol.ISuggestResultDto = {
823824
x: pid,
824-
b: [],
825-
a: { replace: typeConvert.Range.from(replaceRange), insert: typeConvert.Range.from(insertRange) },
826-
c: list.isIncomplete || undefined
825+
[extHostProtocol.ISuggestResultDtoField.completions]: completions,
826+
[extHostProtocol.ISuggestResultDtoField.defaultRanges]: { replace: typeConvert.Range.from(replaceRange), insert: typeConvert.Range.from(insertRange) },
827+
[extHostProtocol.ISuggestResultDtoField.isIncomplete]: list.isIncomplete || undefined
827828
};
828829

829830
for (let i = 0; i < list.items.length; i++) {
830-
const suggestion = this._convertCompletionItem(list.items[i], pos, [pid, i]);
831-
// check for bad completion item
832-
// for the converter did warn
833-
if (suggestion) {
834-
result.b.push(suggestion);
831+
const item = list.items[i];
832+
// check for bad completion item first
833+
if (this._validateCompletionItem(item, pos)) {
834+
const dto = this._convertCompletionItem(item, [pid, i], insertRange, replaceRange);
835+
completions.push(dto);
835836
}
836837
}
837838

@@ -850,12 +851,13 @@ class SuggestAdapter {
850851
return Promise.resolve(undefined);
851852
}
852853

854+
const pos = typeConvert.Position.to(position);
853855
const _mustNotChange = SuggestAdapter._mustNotChangeHash(item);
854856
const _mayNotChange = SuggestAdapter._mayNotChangeHash(item);
855857

856858
return asPromise(() => this._provider.resolveCompletionItem!(item, token)).then(resolvedItem => {
857859

858-
if (!resolvedItem) {
860+
if (!resolvedItem || !this._validateCompletionItem(resolvedItem, pos)) {
859861
return undefined;
860862
}
861863

@@ -885,8 +887,7 @@ class SuggestAdapter {
885887
this._didWarnShould = true;
886888
}
887889

888-
const pos = typeConvert.Position.to(position);
889-
return this._convertCompletionItem(resolvedItem, pos, id);
890+
return this._convertCompletionItem(resolvedItem, id);
890891
});
891892
}
892893

@@ -896,11 +897,7 @@ class SuggestAdapter {
896897
this._cache.delete(id);
897898
}
898899

899-
private _convertCompletionItem(item: vscode.CompletionItem, position: vscode.Position, id: extHostProtocol.ChainedCacheId): extHostProtocol.ISuggestDataDto | undefined {
900-
if (typeof item.label !== 'string' || item.label.length === 0) {
901-
this._logService.warn('INVALID text edit -> must have at least a label');
902-
return undefined;
903-
}
900+
private _convertCompletionItem(item: vscode.CompletionItem, id: extHostProtocol.ChainedCacheId, defaultInsertRange?: vscode.Range, defaultReplaceRange?: vscode.Range): extHostProtocol.ISuggestDataDto {
904901

905902
const disposables = this._disposables.get(id[0]);
906903
if (!disposables) {
@@ -928,9 +925,7 @@ class SuggestAdapter {
928925

929926
// 'insertText'-logic
930927
if (item.textEdit) {
931-
this._apiDeprecation.report('CompletionItem.textEdit', this._extension,
932-
`Use 'CompletionItem.insertText' and 'CompletionItem.range' instead.`);
933-
928+
this._apiDeprecation.report('CompletionItem.textEdit', this._extension, `Use 'CompletionItem.insertText' and 'CompletionItem.range' instead.`);
934929
result[extHostProtocol.ISuggestDataDtoField.insertText] = item.textEdit.newText;
935930

936931
} else if (typeof item.insertText === 'string') {
@@ -949,36 +944,44 @@ class SuggestAdapter {
949944
range = item.range;
950945
}
951946

952-
if (range) {
953-
if (Range.isRange(range)) {
954-
if (!SuggestAdapter._isValidRangeForCompletion(range, position)) {
955-
this._logService.trace('INVALID range -> must be single line and on the same line');
956-
return undefined;
957-
}
958-
result[extHostProtocol.ISuggestDataDtoField.range] = typeConvert.Range.from(range);
947+
if (Range.isRange(range)) {
948+
// "old" range
949+
result[extHostProtocol.ISuggestDataDtoField.range] = typeConvert.Range.from(range);
959950

960-
} else {
961-
if (
962-
!SuggestAdapter._isValidRangeForCompletion(range.inserting, position)
963-
|| !SuggestAdapter._isValidRangeForCompletion(range.replacing, position)
964-
|| !range.inserting.start.isEqual(range.replacing.start)
965-
|| !range.replacing.contains(range.inserting)
966-
) {
967-
this._logService.trace('INVALID range -> must be single line, on the same line, insert range must be a prefix of replace range');
968-
return undefined;
969-
}
970-
result[extHostProtocol.ISuggestDataDtoField.range] = {
971-
insert: typeConvert.Range.from(range.inserting),
972-
replace: typeConvert.Range.from(range.replacing)
973-
};
974-
}
951+
} else if (range && !defaultInsertRange?.isEqual(range.inserting) && !defaultReplaceRange?.isEqual(range.replacing)) {
952+
// ONLY send range when it's different from the default ranges (safe bandwidth)
953+
result[extHostProtocol.ISuggestDataDtoField.range] = {
954+
insert: typeConvert.Range.from(range.inserting),
955+
replace: typeConvert.Range.from(range.replacing)
956+
};
975957
}
976958

977959
return result;
978960
}
979961

980-
private static _isValidRangeForCompletion(range: vscode.Range, position: vscode.Position): boolean {
981-
return range.isSingleLine || range.start.line === position.line;
962+
private _validateCompletionItem(item: vscode.CompletionItem, position: vscode.Position): boolean {
963+
if (typeof item.label !== 'string' || item.label.length === 0) {
964+
this._logService.warn('INVALID text edit -> must have at least a label');
965+
return false;
966+
}
967+
968+
if (Range.isRange(item.range)) {
969+
if (!item.range.isSingleLine || item.range.start.line !== position.line) {
970+
this._logService.trace('INVALID range -> must be single line and on the same line');
971+
return false;
972+
}
973+
974+
} else if (item.range) {
975+
if (!item.range.inserting.isSingleLine || item.range.inserting.start.line !== position.line
976+
|| !item.range.replacing.isSingleLine || item.range.replacing.start.line !== position.line
977+
|| !item.range.inserting.start.isEqual(item.range.replacing.start)
978+
|| !item.range.replacing.contains(item.range.inserting)
979+
) {
980+
this._logService.trace('INVALID range -> must be single line, on the same line, insert range must be a prefix of replace range');
981+
return false;
982+
}
983+
}
984+
return true;
982985
}
983986

984987
private static _mustNotChangeHash(item: vscode.CompletionItem) {

0 commit comments

Comments
 (0)