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

Add support for CompletionList "applyKind" to control how defaults and per-item commitCharacters/data are combined #1558

Merged
merged 6 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
141 changes: 140 additions & 1 deletion client-node-tests/src/converter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { strictEqual, deepEqual, ok } from 'assert';
import { strictEqual, deepEqual, ok, deepStrictEqual } from 'assert';

import * as proto from 'vscode-languageclient';
import * as codeConverter from 'vscode-languageclient/$test/common/codeConverter';
Expand Down Expand Up @@ -859,6 +859,145 @@ suite('Protocol Converter', () => {
ok(result.items[0].insertText instanceof vscode.SnippetString);
});

test('Completion Result - applyKind:default - commitCharacters', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { commitCharacters: ['d'] },
items: [{ label: 'item', commitCharacters: ['i'] }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, ['i']);
});

test('Completion Result - applyKind:replace - commitCharacters', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { commitCharacters: ['1'] },
// Set other fields to "merge" to ensure the correct field was used.
applyKind: { commitCharacters: proto.ApplyKind.Replace, data: proto.ApplyKind.Merge },
items: [{ label: 'item', commitCharacters: ['2'] }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, ['2']);
});

test('Completion Result - applyKind:replace - commitCharacters - item empty', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { commitCharacters: ['1'] },
// Set other fields to "merge" to ensure the correct field was used.
applyKind: { commitCharacters: proto.ApplyKind.Replace, data: proto.ApplyKind.Merge },
items: [{ label: 'item', commitCharacters: [] }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, []);
});

test('Completion Result - applyKind:merge - commitCharacters - both supplied with overlaps', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { commitCharacters: ['d', 'b'] },
applyKind: { commitCharacters: proto.ApplyKind.Merge },
items: [{ label: 'item', commitCharacters: ['b', 'i'] }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, ['d', 'b', 'i']);
});

test('Completion Result - applyKind:merge - commitCharacters - only default supplied', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { commitCharacters: ['d'] },
applyKind: { commitCharacters: proto.ApplyKind.Merge },
items: [{ label: 'item' }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, ['d']);
});

test('Completion Result - applyKind:merge - commitCharacters - only item supplied', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { },
applyKind: { commitCharacters: proto.ApplyKind.Merge },
items: [{ label: 'item', commitCharacters: ['i'] }]
};
const result = await p2c.asCompletionResult(completionResult);
deepStrictEqual(result.items[0].commitCharacters, ['i']);
});

test('Completion Result - applyKind:default - data', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd': 'd' } },
items: [{ label: 'item', data: { 'i': 'i' } }]
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'i': 'i'});
});

test('Completion Result - applyKind:replace - data', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd': 'd' } },
// Set other fields to "merge" to ensure the correct field was used.
applyKind: { data: proto.ApplyKind.Replace, commitCharacters: proto.ApplyKind.Merge },
items: [{ label: 'item', data: { 'i': 'i' } }]
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'i': 'i'});
});

test('Completion Result - applyKind:merge - data - both supplied', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd': 'd' } },
applyKind: { data: proto.ApplyKind.Merge },
items: [{ label: 'item', data: { 'i': 'i' } }]
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'d': 'd', 'i': 'i'});
});

test('Completion Result - applyKind:merge - data - default supplied, item null', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd': 'd' } },
applyKind: { data: proto.ApplyKind.Merge },
items: [{ label: 'item', data: null }] // null treated like undefined
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'d': 'd'}); // gets default
});

test('Completion Result - applyKind:merge - data - both supplied, item has null fields', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd': 'd' } },
applyKind: { data: proto.ApplyKind.Merge },
items: [{ label: 'item', data: { 'd': null, 'i': 'i'} }] // null treated like undefined
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'d': 'd', 'i': 'i'}); // gets default for 'd'
});

test('Completion Result - applyKind:merge - data - both supplied, item has non-null falsy fields', async () => {
const completionResult: proto.CompletionList = {
isIncomplete: false,
itemDefaults: { data: { 'd1': 'd1', 'd2': 'd2' } },
applyKind: { data: proto.ApplyKind.Merge },
items: [{ label: 'item', data: { 'd1': 0, 'd2': ''} }] // Both falsy, but should be used.
};
const result = await p2c.asCompletionResult(completionResult);
const protoResult = await c2p.asCompletionItem(result.items[0]);
deepStrictEqual(protoResult.data, {'d1': 0, 'd2': ''});
});

test('Parameter Information', async () => {
const parameterInfo: proto.ParameterInformation = {
label: 'label'
Expand Down
5 changes: 3 additions & 2 deletions client/src/common/completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ export class CompletionItemFeature extends TextDocumentLanguageFeature<Completio
completion.completionList = {
itemDefaults: [
'commitCharacters', 'editRange', 'insertTextFormat', 'insertTextMode', 'data'
]
],
applyKindSupport: true,
};
}

Expand Down Expand Up @@ -153,4 +154,4 @@ export class CompletionItemFeature extends TextDocumentLanguageFeature<Completio
};
return [Languages.registerCompletionItemProvider(this._client.protocol2CodeConverter.asDocumentSelector(selector), provider, ...triggerCharacters), provider];
}
}
}
63 changes: 57 additions & 6 deletions client/src/common/protocolConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ export function createConverter(
const list = <ls.CompletionList>value;
const { defaultRange, commitCharacters } = getCompletionItemDefaults(list, allCommitCharacters);
const converted = await async.map(list.items, (item) => {
return asCompletionItem(item, commitCharacters, defaultRange, list.itemDefaults?.insertTextMode, list.itemDefaults?.insertTextFormat, list.itemDefaults?.data);
return asCompletionItem(item, commitCharacters, list.applyKind?.commitCharacters, defaultRange, list.itemDefaults?.insertTextMode, list.itemDefaults?.insertTextFormat, list.itemDefaults?.data, list.applyKind?.data);
}, token);
return new code.CompletionList(converted, list.isIncomplete);
}
Expand Down Expand Up @@ -561,7 +561,16 @@ export function createConverter(
return result;
}

function asCompletionItem(item: ls.CompletionItem, defaultCommitCharacters?: string[], defaultRange?: code.Range | InsertReplaceRange, defaultInsertTextMode?: ls.InsertTextMode, defaultInsertTextFormat?: ls.InsertTextFormat, defaultData?: ls.LSPAny): ProtocolCompletionItem {
function asCompletionItem(
item: ls.CompletionItem,
defaultCommitCharacters?: string[],
commitCharactersApplyKind?: ls.ApplyKind,
defaultRange?: code.Range | InsertReplaceRange,
defaultInsertTextMode?: ls.InsertTextMode,
defaultInsertTextFormat?: ls.InsertTextFormat,
defaultData?: ls.LSPAny,
dataApplyKind?: ls.ApplyKind,
): ProtocolCompletionItem {
const tags: code.CompletionItemTag[] = asCompletionItemTags(item.tags);
const label = asCompletionItemLabel(item);
const result = new ProtocolCompletionItem(label);
Expand All @@ -587,9 +596,7 @@ export function createConverter(
}
if (item.sortText) { result.sortText = item.sortText; }
if (item.additionalTextEdits) { result.additionalTextEdits = asTextEditsSync(item.additionalTextEdits); }
const commitCharacters = item.commitCharacters !== undefined
? Is.stringArray(item.commitCharacters) ? item.commitCharacters : undefined
: defaultCommitCharacters;
const commitCharacters = applyCommitCharacters(item, defaultCommitCharacters, commitCharactersApplyKind);
if (commitCharacters) { result.commitCharacters = commitCharacters.slice(); }
if (item.command) { result.command = asCommand(item.command); }
if (item.deprecated === true || item.deprecated === false) {
Expand All @@ -599,7 +606,7 @@ export function createConverter(
}
}
if (item.preselect === true || item.preselect === false) { result.preselect = item.preselect; }
const data = item.data ?? defaultData;
const data = applyData(item, defaultData, dataApplyKind);
if (data !== undefined) { result.data = data; }
if (tags.length > 0) {
result.tags = tags;
Expand All @@ -614,6 +621,50 @@ export function createConverter(
return result;
}

function applyCommitCharacters(item: ls.CompletionItem, defaultCommitCharacters: string[] | undefined, applyKind: ls.ApplyKind | undefined): string[] | undefined {
if (applyKind === ls.ApplyKind.Merge) {
if (!defaultCommitCharacters && !item.commitCharacters) {
return undefined;
}
const set = new Set<string>();
if (defaultCommitCharacters) {
for (const char of defaultCommitCharacters) {
set.add(char);
}
}
if (Is.stringArray(item.commitCharacters)) {
for (const char of item.commitCharacters) {
set.add(char);
}
}
return Array.from(set);
}

return item.commitCharacters !== undefined
? Is.stringArray(item.commitCharacters) ? item.commitCharacters : undefined
: defaultCommitCharacters;
}

function applyData(item: ls.CompletionItem, defaultData: any, applyKind: ls.ApplyKind | undefined): string[] | undefined {
if (applyKind === ls.ApplyKind.Merge) {
const data = {
...defaultData,
};

if (item.data) {
Object.entries(item.data).forEach(([key, value]) => {
if (value !== undefined && value !== null) {
data[key] = value;
}
});
}

return data;
}

return item.data ?? defaultData;
}

function asCompletionItemLabel(item: ls.CompletionItem): code.CompletionItemLabel | string {
if (ls.CompletionItemLabelDetails.is(item.labelDetails)) {
return {
Expand Down
72 changes: 70 additions & 2 deletions protocol/metaModel.json
Original file line number Diff line number Diff line change
Expand Up @@ -5168,9 +5168,19 @@
"name": "CompletionItemDefaults"
},
"optional": true,
"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value the one from the item is used.\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value, the rules for combining these are\ndefined by `applyKinds` (if the client supports it), defaulting to\n\"replace\".\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
"since": "3.17.0"
},
{
"name": "applyKind",
"type": {
"kind": "reference",
"name": "CompletionItemApplyKinds"
},
"optional": true,
"documentation": "Specifies how fields from a completion item should be combined with those\nfrom `completionList.itemDefaults`.\n\nIf unspecified, all fields will be treated as \"replace\".\n\nIf a field's value is \"replace\", the value from a completion item (if\nprovided and not `null`) will always be used instead of the value from\n`completionItem.itemDefaults`.\n\nIf a field's value is \"merge\", the values will be merged using the rules\ndefined against each field below.\n\nServers are only allowed to return `applyKind` if the client\nsignals support for this via the `completionList.applyKindSupport`\ncapability.\n\n@since 3.18.0",
"since": "3.18.0"
},
{
"name": "items",
"type": {
Expand Down Expand Up @@ -9195,9 +9205,36 @@
"since": "3.17.0"
}
],
"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value the one from the item is used.\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
"documentation": "In many cases the items of an actual completion result share the same\nvalue for properties like `commitCharacters` or the range of a text\nedit. A completion list can therefore define item defaults which will\nbe used if a completion item itself doesn't specify the value.\n\nIf a completion list specifies a default value and a completion item\nalso specifies a corresponding value, the rules for combining these are\ndefined by `applyKinds` (if the client supports it), defaulting to\n\"replace\".\n\nServers are only allowed to return default values if the client\nsignals support for this via the `completionList.itemDefaults`\ncapability.\n\n@since 3.17.0",
"since": "3.17.0"
},
{
"name": "CompletionItemApplyKinds",
"properties": [
{
"name": "commitCharacters",
"type": {
"kind": "reference",
"name": "ApplyKind"
},
"optional": true,
"documentation": "Specifies whether commitCharacters on a completion will replace or be\nmerged with those in `completionList.itemDefaults.commitCharacters`.\n\nIf \"replace\", the commit characters from the completion item will\nalways be used unless not provided, in which case those from\n`completionList.itemDefaults.commitCharacters` will be used. An\nempty list can be used if a completion item does not have any commit\ncharacters and also should not use those from\n`completionList.itemDefaults.commitCharacters`.\n\nIf \"merge\" the commitCharacters for the completion will be the union\nof all values in both `completionList.itemDefaults.commitCharacters`\nand the completion's own `commitCharacters`.\n\n@since 3.18.0",
"since": "3.18.0"
},
{
"name": "data",
"type": {
"kind": "reference",
"name": "ApplyKind"
},
"optional": true,
"documentation": "Specifies whether the `data` field on a completion will replace or\nbe merged with data from `completionList.itemDefaults.data`.\n\nIf \"replace\", the data from the completion item will be used if\nprovided (and not `null`), otherwise\n`completionList.itemDefaults.data` will be used. An empty object can\nbe used if a completion item does not have any data but also should\nnot use the value from `completionList.itemDefaults.data`.\n\nIf \"merge\", a shallow merge will be performed between\n`completionList.itemDefaults.data` and the completion's own data\nusing the following rules:\n\n- If a completion's `data` field is not provided (or `null`), the\n entire `data` field from `completionList.itemDefaults.data` will be\n used as-is.\n- If a completion's `data` field is provided, each field will\n overwrite the field of the same name in\n `completionList.itemDefaults.data` but no merging of nested fields\n within that value will occur.\n\n@since 3.18.0",
"since": "3.18.0"
}
],
"documentation": "Specifies how fields from a completion item should be combined with those\nfrom `completionList.itemDefaults`.\n\nIf unspecified, all fields will be treated as \"replace\".\n\nIf a field's value is \"replace\", the value from a completion item (if\nprovided and not `null`) will always be used instead of the value from\n`completionItem.itemDefaults`.\n\nIf a field's value is \"merge\", the values will be merged using the rules\ndefined against each field below.\n\nServers are only allowed to return `applyKind` if the client\nsignals support for this via the `completionList.applyKindSupport`\ncapability.\n\n@since 3.18.0",
"since": "3.18.0"
},
{
"name": "CompletionOptions",
"properties": [
Expand Down Expand Up @@ -13560,6 +13597,16 @@
"optional": true,
"documentation": "The client supports the following itemDefaults on\na completion list.\n\nThe value lists the supported property names of the\n`CompletionList.itemDefaults` object. If omitted\nno properties are supported.\n\n@since 3.17.0",
"since": "3.17.0"
},
{
"name": "applyKindSupport",
"type": {
"kind": "base",
"name": "boolean"
},
"optional": true,
"documentation": "Specifies whether the client supports `CompletionList.applyKind` to\nindicate how supported values from `completionList.itemDefaults`\nand `completion` will be combined.\n\nIf a client supports `applyKind` it must support it for all fields\nthat it supports that are listed in `CompletionList.applyKind`. This\nmeans when clients add support for new/future fields in completion\nitems the MUST also support merge for them if those fields are\ndefined in `CompletionList.applyKind`.\n\n@since 3.18.0",
"since": "3.18.0"
}
],
"documentation": "The client supports the following `CompletionList` specific\ncapabilities.\n\n@since 3.17.0",
Expand Down Expand Up @@ -15314,6 +15361,27 @@
],
"documentation": "How a completion was triggered"
},
{
"name": "ApplyKind",
"type": {
"kind": "base",
"name": "string"
},
"values": [
{
"name": "Replace",
"value": "replace",
"documentation": "The value from the individual item (if provided and not `null`) will be\nused instead of the default."
},
{
"name": "Merge",
"value": "merge",
"documentation": "The value from the item will be merged with the default.\n\nThe specific rules for mergeing values are defined against each field\nthat supports merging."
}
],
"documentation": "Defines how values from a set of defaults and an individual item will be\nmerged.\n\n@since 3.18.0",
"since": "3.18.0"
},
{
"name": "SignatureHelpTriggerKind",
"type": {
Expand Down
Loading