Skip to content

Commit

Permalink
Use ChatAgentService instead of InlineChatService for terminal ch…
Browse files Browse the repository at this point in the history 
…at hint (#213183)

fix #213159
  • Loading branch information
@meganrogge
meganrogge authored May 21, 2024
1 parent ba9fe55 commit c0c8cdd
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 28 deletions.
31 changes: 18 additions & 13 deletions src/vs/workbench/contrib/terminalContrib/chat/browser/terminal.initialHint.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
import { ICommandService } from 'vs/platform/commands/common/commands';
import { IProductService } from 'vs/platform/product/common/productService';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IInlineChatService, IInlineChatSessionProvider, InlineChatProviderChangeEvent } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
import { status } from 'vs/base/browser/ui/aria/aria';
import * as dom from 'vs/base/browser/dom';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { TerminalChatCommandId } from 'vs/workbench/contrib/terminalContrib/chat/browser/terminalChat';
import { TerminalInstance } from 'vs/workbench/contrib/terminal/browser/terminalInstance';
import 'vs/css!./media/terminalInitialHint';
import { TerminalInitialHintSettingId } from 'vs/workbench/contrib/terminalContrib/chat/common/terminalInitialHintConfiguration';
import { ChatAgentLocation, IChatAgent, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';

const $ = dom.$;

Expand All @@ -38,7 +38,7 @@ export class InitialHintAddon extends Disposable implements ITerminalAddon {
private readonly _disposables = this._register(new MutableDisposable<DisposableStore>());

constructor(private readonly _capabilities: ITerminalCapabilityStore,
private readonly _onDidChangeProviders: Event<InlineChatProviderChangeEvent>) {
private readonly _onDidChangeAgents: Event<IChatAgent | undefined>) {
super();
}
activate(terminal: RawXtermTerminal): void {
Expand All @@ -58,8 +58,13 @@ export class InitialHintAddon extends Disposable implements ITerminalAddon {
}
}));
}

this._disposables.value?.add(Event.once(this._onDidChangeProviders)(() => this._onDidRequestCreateHint.fire()));
const agentListener = this._onDidChangeAgents((e) => {
if (e?.locations.includes(ChatAgentLocation.Terminal)) {
this._onDidRequestCreateHint.fire();
agentListener.dispose();
}
});
this._disposables.value?.add(agentListener);
}
}

Expand All @@ -80,11 +85,11 @@ export class TerminalInitialHintContribution extends Disposable implements ITerm
private readonly _instance: Pick<ITerminalInstance, 'capabilities'> | IDetachedTerminalInstance,
processManager: ITerminalProcessManager | ITerminalProcessInfo | undefined,
widgetManager: TerminalWidgetManager | undefined,
@IInlineChatService private readonly _inlineChatService: IInlineChatService,
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
@ITerminalGroupService private readonly _terminalGroupService: ITerminalGroupService,
@ITerminalEditorService private readonly _terminalEditorService: ITerminalEditorService,
@IChatAgentService private readonly _chatAgentService: IChatAgentService,
) {
super();
}
Expand All @@ -95,7 +100,7 @@ export class TerminalInitialHintContribution extends Disposable implements ITerm
return;
}
this._xterm = xterm;
this._addon = this._register(this._instantiationService.createInstance(InitialHintAddon, this._instance.capabilities, this._inlineChatService.onDidChangeProviders));
this._addon = this._register(this._instantiationService.createInstance(InitialHintAddon, this._instance.capabilities, this._chatAgentService.onDidChangeAgents));
this._xterm.raw.loadAddon(this._addon);
this._register(this._addon.onDidRequestCreateHint(() => this._createHint()));
}
Expand Down Expand Up @@ -148,11 +153,11 @@ export class TerminalInitialHintContribution extends Disposable implements ITerm
this._register(this._decoration);
this._register(this._decoration.onRender((e) => {
if (!this._hintWidget && this._xterm?.isFocused && this._terminalGroupService.instances.length + this._terminalEditorService.instances.length === 1) {
const chatProviders = [...this._inlineChatService.getAllProvider()];
if (chatProviders?.length) {
const terminalAgents = this._chatAgentService.getActivatedAgents().filter(candidate => candidate.locations.includes(ChatAgentLocation.Terminal));
if (terminalAgents?.length) {
const widget = this._register(this._instantiationService.createInstance(TerminalInitialHintWidget, instance));
this._addon?.dispose();
this._hintWidget = widget.getDomNode(chatProviders);
this._hintWidget = widget.getDomNode(terminalAgents);
if (!this._hintWidget) {
return;
}
Expand Down Expand Up @@ -213,8 +218,8 @@ class TerminalInitialHintWidget extends Disposable {
}));
}

private _getHintInlineChat(providers: IInlineChatSessionProvider[]) {
const providerName = (providers.length === 1 ? providers[0].label : undefined) ?? this.productService.nameShort;
private _getHintInlineChat(agents: IChatAgent[]) {
const providerName = (agents.length === 1 ? agents[0].fullName : undefined) ?? this.productService.nameShort;

let ariaLabel = `Ask ${providerName} something or start typing to dismiss.`;

Expand Down Expand Up @@ -289,12 +294,12 @@ class TerminalInitialHintWidget extends Disposable {
return { ariaLabel, hintHandler, hintElement };
}

getDomNode(providers: IInlineChatSessionProvider[]): HTMLElement {
getDomNode(agents: IChatAgent[]): HTMLElement {
if (!this.domNode) {
this.domNode = $('.terminal-initial-hint');
this.domNode!.style.paddingLeft = '4px';

const { hintElement, ariaLabel } = this._getHintInlineChat(providers);
const { hintElement, ariaLabel } = this._getHintInlineChat(agents);
this.domNode.append(hintElement);
this.ariaLabel = ariaLabel.concat(localize('disableHint', ' Toggle {0} in settings to disable this hint.', AccessibilityVerbositySettingId.TerminalChat));

Expand Down
60 changes: 45 additions & 15 deletions src/vs/workbench/contrib/terminalContrib/chat/test/browser/terminalInitialHint.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { workbenchInstantiationService } from 'vs/workbench/test/browser/workben
import { NullLogService } from 'vs/platform/log/common/log';
import { InitialHintAddon } from 'vs/workbench/contrib/terminalContrib/chat/browser/terminal.initialHint.contribution';
import { getActiveDocument } from 'vs/base/browser/dom';
import { IInlineChatSessionProvider, InlineChatProviderChangeEvent } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
import { Emitter } from 'vs/base/common/event';
import { strictEqual } from 'assert';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { ChatAgentLocation, IChatAgent } from 'vs/workbench/contrib/chat/common/chatAgents';

// Test TerminalInitialHintAddon

Expand All @@ -22,13 +22,35 @@ suite('Terminal Initial Hint Addon', () => {
let eventCount = 0;
let xterm: Terminal;
let initialHintAddon: InitialHintAddon;
const _onDidChangeProviders: Emitter<InlineChatProviderChangeEvent> = new Emitter();
const onDidChangeProviders = _onDidChangeProviders.event;
const _onDidChangeAgents: Emitter<IChatAgent | undefined> = new Emitter();
const onDidChangeAgents = _onDidChangeAgents.event;
const agent: IChatAgent = {
id: 'termminal',
name: 'terminal',
extensionId: new ExtensionIdentifier('test'),
extensionPublisherId: 'test',
extensionDisplayName: 'test',
metadata: {},
slashCommands: [{ name: 'test', description: 'test' }],
locations: [ChatAgentLocation.fromRaw('terminal')],
invoke: async () => { return {}; }
};
const editorAgent: IChatAgent = {
id: 'editor',
name: 'editor',
extensionId: new ExtensionIdentifier('test-editor'),
extensionPublisherId: 'test-editor',
extensionDisplayName: 'test-editor',
metadata: {},
slashCommands: [{ name: 'test', description: 'test' }],
locations: [ChatAgentLocation.fromRaw('editor')],
invoke: async () => { return {}; }
};
setup(() => {
const instantiationService = workbenchInstantiationService({}, store);
xterm = store.add(new Terminal());
const shellIntegrationAddon = store.add(new ShellIntegrationAddon('', true, undefined, new NullLogService));
initialHintAddon = store.add(instantiationService.createInstance(InitialHintAddon, shellIntegrationAddon.capabilities, onDidChangeProviders));
initialHintAddon = store.add(instantiationService.createInstance(InitialHintAddon, shellIntegrationAddon.capabilities, onDidChangeAgents));
store.add(initialHintAddon.onDidRequestCreateHint(() => eventCount++));
const testContainer = document.createElement('div');
getActiveDocument().body.append(testContainer);
Expand All @@ -44,24 +66,32 @@ suite('Terminal Initial Hint Addon', () => {
xterm.focus();
strictEqual(eventCount, 0);
});
test('hint is shown when there is a chat provider', () => {
test('hint is not shown when there is just an editor agent', () => {
eventCount = 0;
const provider: IInlineChatSessionProvider = {
extensionId: new ExtensionIdentifier('test'),
label: 'blahblah'
};
_onDidChangeProviders.fire({ added: provider });
_onDidChangeAgents.fire(editorAgent);
xterm.focus();
strictEqual(eventCount, 0);
});
test('hint is shown when there is a terminal chat agent', () => {
eventCount = 0;
_onDidChangeAgents.fire(editorAgent);
xterm.focus();
strictEqual(eventCount, 0);
_onDidChangeAgents.fire(agent);
strictEqual(eventCount, 1);
});
test('hint is not shown again when another terminal chat agent is added if it has already shown', () => {
eventCount = 0;
_onDidChangeAgents.fire(agent);
xterm.focus();
strictEqual(eventCount, 1);
_onDidChangeAgents.fire(agent);
strictEqual(eventCount, 1);
});
});
suite('Input', () => {
test('hint is not shown when there has been input', () => {
const provider: IInlineChatSessionProvider = {
extensionId: new ExtensionIdentifier('test'),
label: 'blahblah'
};
_onDidChangeProviders.fire({ added: provider });
_onDidChangeAgents.fire(agent);
xterm.writeln('data');
setTimeout(() => {
xterm.focus();
Expand Down

0 comments on commit c0c8cdd

Please sign in to comment.