Skip to content

Commit edb9125

Browse files
CopilotTyriar
andcommitted
Fix terminal suggest providers default enablement logic
Co-authored-by: Tyriar <[email protected]>
1 parent a1fc17e commit edb9125

File tree

3 files changed

+179
-1
lines changed

3 files changed

+179
-1
lines changed

src/vs/workbench/contrib/terminalContrib/suggest/browser/terminalCompletionService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
156156
const providerConfig: { [key: string]: boolean } = this._configurationService.getValue(TerminalSuggestSettingId.Providers);
157157
providers = providers.filter(p => {
158158
const providerId = p.id;
159-
return providerId && providerId in providerConfig && providerConfig[providerId] !== false;
159+
return providerId && (!(providerId in providerConfig) || providerConfig[providerId] !== false);
160160
});
161161

162162
if (!providers.length) {

src/vs/workbench/contrib/terminalContrib/suggest/common/terminalSuggestConfiguration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export interface ITerminalSuggestConfiguration {
5757
providers: {
5858
'terminal-suggest': boolean;
5959
'pwsh-shell-integration': boolean;
60+
[key: string]: boolean;
6061
};
6162
showStatusBar: boolean;
6263
cdPath: 'off' | 'relative' | 'absolute';
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { CancellationToken } from '../../../../../../base/common/cancellation.js';
8+
import { IDisposable } from '../../../../../../base/common/lifecycle.js';
9+
import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
10+
import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js';
11+
import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
12+
import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js';
13+
import { TerminalCapabilityStore } from '../../../../../../platform/terminal/common/capabilities/terminalCapabilityStore.js';
14+
import { TerminalShellType } from '../../../../../../platform/terminal/common/terminal.js';
15+
import { ITerminalCompletionProvider, TerminalCompletionService } from '../../browser/terminalCompletionService.js';
16+
import { ITerminalCompletion, TerminalCompletionItemKind } from '../../browser/terminalCompletionItem.js';
17+
import { TerminalSuggestSettingId } from '../../common/terminalSuggestConfiguration.js';
18+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
19+
20+
suite('TerminalCompletionService - Provider Configuration', () => {
21+
let instantiationService: TestInstantiationService;
22+
let configurationService: TestConfigurationService;
23+
let terminalCompletionService: TerminalCompletionService;
24+
let capabilities: TerminalCapabilityStore;
25+
const disposables: IDisposable[] = [];
26+
27+
ensureNoDisposablesAreLeakedInTestSuite();
28+
29+
setup(() => {
30+
instantiationService = new TestInstantiationService();
31+
configurationService = new TestConfigurationService();
32+
instantiationService.stub(IConfigurationService, configurationService);
33+
instantiationService.stub(ILogService, new NullLogService());
34+
35+
terminalCompletionService = instantiationService.createInstance(TerminalCompletionService);
36+
capabilities = new TerminalCapabilityStore();
37+
});
38+
39+
teardown(() => {
40+
disposables.forEach(d => d.dispose());
41+
disposables.length = 0;
42+
terminalCompletionService.dispose();
43+
});
44+
45+
// Mock provider for testing
46+
function createMockProvider(id: string): ITerminalCompletionProvider {
47+
return {
48+
id,
49+
provideCompletions: async () => [{
50+
label: `completion-from-${id}`,
51+
kind: TerminalCompletionItemKind.Method,
52+
replacementIndex: 0,
53+
replacementLength: 0
54+
}]
55+
};
56+
}
57+
58+
test('should enable providers by default when no configuration exists', async () => {
59+
// Register two providers - one in default config, one not
60+
const defaultProvider = createMockProvider('terminal-suggest');
61+
const newProvider = createMockProvider('new-extension-provider');
62+
63+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'terminal-suggest', defaultProvider));
64+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'new-extension-provider', newProvider));
65+
66+
// Set empty configuration (no provider keys)
67+
configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {});
68+
69+
const result = await terminalCompletionService.provideCompletions(
70+
'test',
71+
4,
72+
false,
73+
TerminalShellType.Bash,
74+
capabilities,
75+
CancellationToken.None
76+
);
77+
78+
// Both providers should be enabled since they're not explicitly disabled
79+
assert.ok(result, 'Should have completions');
80+
assert.strictEqual(result.length, 2, 'Should have completions from both providers');
81+
82+
const labels = result.map(c => c.label);
83+
assert.ok(labels.includes('completion-from-terminal-suggest'), 'Should include completion from default provider');
84+
assert.ok(labels.includes('completion-from-new-extension-provider'), 'Should include completion from new provider');
85+
});
86+
87+
test('should disable providers when explicitly set to false', async () => {
88+
const provider1 = createMockProvider('provider1');
89+
const provider2 = createMockProvider('provider2');
90+
91+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider1', provider1));
92+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider2', provider2));
93+
94+
// Disable provider1, leave provider2 unconfigured
95+
configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
96+
'provider1': false
97+
});
98+
99+
const result = await terminalCompletionService.provideCompletions(
100+
'test',
101+
4,
102+
false,
103+
TerminalShellType.Bash,
104+
capabilities,
105+
CancellationToken.None
106+
);
107+
108+
// Only provider2 should be enabled
109+
assert.ok(result, 'Should have completions');
110+
assert.strictEqual(result.length, 1, 'Should have completions from only one provider');
111+
assert.strictEqual(result[0].label, 'completion-from-provider2', 'Should only include completion from enabled provider');
112+
});
113+
114+
test('should enable providers when explicitly set to true', async () => {
115+
const provider1 = createMockProvider('provider1');
116+
const provider2 = createMockProvider('provider2');
117+
118+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider1', provider1));
119+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider2', provider2));
120+
121+
// Explicitly enable provider1, leave provider2 unconfigured
122+
configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
123+
'provider1': true
124+
});
125+
126+
const result = await terminalCompletionService.provideCompletions(
127+
'test',
128+
4,
129+
false,
130+
TerminalShellType.Bash,
131+
capabilities,
132+
CancellationToken.None
133+
);
134+
135+
// Both providers should be enabled
136+
assert.ok(result, 'Should have completions');
137+
assert.strictEqual(result.length, 2, 'Should have completions from both providers');
138+
139+
const labels = result.map(c => c.label);
140+
assert.ok(labels.includes('completion-from-provider1'), 'Should include completion from explicitly enabled provider');
141+
assert.ok(labels.includes('completion-from-provider2'), 'Should include completion from default enabled provider');
142+
});
143+
144+
test('should handle mixed configuration correctly', async () => {
145+
const provider1 = createMockProvider('provider1');
146+
const provider2 = createMockProvider('provider2');
147+
const provider3 = createMockProvider('provider3');
148+
149+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider1', provider1));
150+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider2', provider2));
151+
disposables.push(terminalCompletionService.registerTerminalCompletionProvider('test-ext', 'provider3', provider3));
152+
153+
// Mixed configuration: enable provider1, disable provider2, leave provider3 unconfigured
154+
configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
155+
'provider1': true,
156+
'provider2': false
157+
});
158+
159+
const result = await terminalCompletionService.provideCompletions(
160+
'test',
161+
4,
162+
false,
163+
TerminalShellType.Bash,
164+
capabilities,
165+
CancellationToken.None
166+
);
167+
168+
// provider1 and provider3 should be enabled, provider2 should be disabled
169+
assert.ok(result, 'Should have completions');
170+
assert.strictEqual(result.length, 2, 'Should have completions from two providers');
171+
172+
const labels = result.map(c => c.label);
173+
assert.ok(labels.includes('completion-from-provider1'), 'Should include completion from explicitly enabled provider');
174+
assert.ok(labels.includes('completion-from-provider3'), 'Should include completion from default enabled provider');
175+
assert.ok(!labels.includes('completion-from-provider2'), 'Should not include completion from disabled provider');
176+
});
177+
});

0 commit comments

Comments
 (0)