Extract provider filtering logic into protected method and move tests to main test file

Co-authored-by: Tyriar <[email protected]>

Author: Copilot

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

@@ -153,11 +153,7 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
 			return this._collectCompletions(providers, shellType, promptValue, cursorPosition, allowFallbackCompletions, capabilities, token, explicitlyInvoked);
 		}
 
-		const providerConfig: { [key: string]: boolean } = this._configurationService.getValue(TerminalSuggestSettingId.Providers);
-		providers = providers.filter(p => {
-			const providerId = p.id;
-			return providerId && (!(providerId in providerConfig) || providerConfig[providerId] !== false);
-		});
+		providers = this._getEnabledProviders(providers);
 
 		if (!providers.length) {
 			return;
@@ -166,6 +162,14 @@ export class TerminalCompletionService extends Disposable implements ITerminalCo
 		return this._collectCompletions(providers, shellType, promptValue, cursorPosition, allowFallbackCompletions, capabilities, token, explicitlyInvoked);
 	}
 
+	protected _getEnabledProviders(providers: ITerminalCompletionProvider[]): ITerminalCompletionProvider[] {
+		const providerConfig: { [key: string]: boolean } = this._configurationService.getValue(TerminalSuggestSettingId.Providers);
+		return providers.filter(p => {
+			const providerId = p.id;
+			return providerId && (!(providerId in providerConfig) || providerConfig[providerId] !== false);
+		});
+	}
+
 	private async _collectCompletions(providers: ITerminalCompletionProvider[], shellType: TerminalShellType | undefined, promptValue: string, cursorPosition: number, allowFallbackCompletions: boolean, capabilities: ITerminalCapabilityStore, token: CancellationToken, explicitlyInvoked?: boolean): Promise<ITerminalCompletion[] | undefined> {
 		const completionPromises = providers.map(async provider => {
 			if (provider.shellTypes && shellType && !provider.shellTypes.includes(shellType)) {

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

@@ -770,4 +770,105 @@ suite('TerminalCompletionService', () => {
 		});
 
 	});
+
+	suite('Provider Configuration', () => {
+		// Test class that extends TerminalCompletionService to access protected methods
+		class TestTerminalCompletionService extends TerminalCompletionService {
+			public _getEnabledProviders(providers: ITerminalCompletionProvider[]): ITerminalCompletionProvider[] {
+				return super._getEnabledProviders(providers);
+			}
+		}
+
+		let testTerminalCompletionService: TestTerminalCompletionService;
+
+		setup(() => {
+			testTerminalCompletionService = store.add(instantiationService.createInstance(TestTerminalCompletionService));
+		});
+
+		// Mock provider for testing
+		function createMockProvider(id: string): ITerminalCompletionProvider {
+			return {
+				id,
+				provideCompletions: async () => [{
+					label: `completion-from-${id}`,
+					kind: TerminalCompletionItemKind.Method,
+					replacementIndex: 0,
+					replacementLength: 0
+				}]
+			};
+		}
+
+		test('should enable providers by default when no configuration exists', () => {
+			const defaultProvider = createMockProvider('terminal-suggest');
+			const newProvider = createMockProvider('new-extension-provider');
+			const providers = [defaultProvider, newProvider];
+
+			// Set empty configuration (no provider keys)
+			configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {});
+
+			const result = testTerminalCompletionService._getEnabledProviders(providers);
+
+			// Both providers should be enabled since they're not explicitly disabled
+			assert.strictEqual(result.length, 2, 'Should enable both providers by default');
+			assert.ok(result.includes(defaultProvider), 'Should include default provider');
+			assert.ok(result.includes(newProvider), 'Should include new provider');
+		});
+
+		test('should disable providers when explicitly set to false', () => {
+			const provider1 = createMockProvider('provider1');
+			const provider2 = createMockProvider('provider2');
+			const providers = [provider1, provider2];
+
+			// Disable provider1, leave provider2 unconfigured
+			configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
+				'provider1': false
+			});
+
+			const result = testTerminalCompletionService._getEnabledProviders(providers);
+
+			// Only provider2 should be enabled
+			assert.strictEqual(result.length, 1, 'Should enable only one provider');
+			assert.ok(result.includes(provider2), 'Should include unconfigured provider');
+			assert.ok(!result.includes(provider1), 'Should not include disabled provider');
+		});
+
+		test('should enable providers when explicitly set to true', () => {
+			const provider1 = createMockProvider('provider1');
+			const provider2 = createMockProvider('provider2');
+			const providers = [provider1, provider2];
+
+			// Explicitly enable provider1, leave provider2 unconfigured
+			configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
+				'provider1': true
+			});
+
+			const result = testTerminalCompletionService._getEnabledProviders(providers);
+
+			// Both providers should be enabled
+			assert.strictEqual(result.length, 2, 'Should enable both providers');
+			assert.ok(result.includes(provider1), 'Should include explicitly enabled provider');
+			assert.ok(result.includes(provider2), 'Should include unconfigured provider');
+		});
+
+		test('should handle mixed configuration correctly', () => {
+			const provider1 = createMockProvider('provider1');
+			const provider2 = createMockProvider('provider2');
+			const provider3 = createMockProvider('provider3');
+			const providers = [provider1, provider2, provider3];
+
+			// Mixed configuration: enable provider1, disable provider2, leave provider3 unconfigured
+			configurationService.setUserConfiguration(TerminalSuggestSettingId.Providers, {
+				'provider1': true,
+				'provider2': false
+			});
+
+			const result = testTerminalCompletionService._getEnabledProviders(providers);
+
+			// provider1 and provider3 should be enabled, provider2 should be disabled
+			assert.strictEqual(result.length, 2, 'Should enable two providers');
+			assert.ok(result.includes(provider1), 'Should include explicitly enabled provider');
+			assert.ok(result.includes(provider3), 'Should include unconfigured provider');
+			assert.ok(!result.includes(provider2), 'Should not include disabled provider');
+		});
+	});
 });