Skip to content

Commit 02d806d

Browse files
justschenjustschen
authored
revert to old implicit attachment flow (#251631)
* revert to old implicit attachment flow * cleanup unused
1 parent 4fc2371 commit 02d806d

File tree

4 files changed

+72
-55
lines changed

4 files changed

+72
-55
lines changed

‎src/vs/workbench/contrib/chat/browser/attachments/implicitContextAttachment.ts

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as dom from '../../../../../base/browser/dom.js';
7-
import { StandardKeyboardEvent } from '../../../../../base/browser/keyboardEvent.js';
87
import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js';
8+
import { Button } from '../../../../../base/browser/ui/button/button.js';
9+
import { getDefaultHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegateFactory.js';
910
import { Codicon } from '../../../../../base/common/codicons.js';
10-
import { KeyCode } from '../../../../../base/common/keyCodes.js';
1111
import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js';
1212
import { Schemas } from '../../../../../base/common/network.js';
1313
import { basename, dirname } from '../../../../../base/common/resources.js';
@@ -21,12 +21,11 @@ import { IMenuService, MenuId } from '../../../../../platform/actions/common/act
2121
import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
2222
import { IContextMenuService } from '../../../../../platform/contextview/browser/contextView.js';
2323
import { FileKind, IFileService } from '../../../../../platform/files/common/files.js';
24+
import { IHoverService } from '../../../../../platform/hover/browser/hover.js';
2425
import { ILabelService } from '../../../../../platform/label/common/label.js';
2526
import { ResourceLabels } from '../../../../browser/labels.js';
2627
import { ResourceContextKey } from '../../../../common/contextkeys.js';
2728
import { IChatRequestImplicitVariableEntry } from '../../common/chatModel.js';
28-
import { IChatWidgetService } from '../chat.js';
29-
import { ChatAttachmentModel } from '../chatAttachmentModel.js';
3029

3130
export class ImplicitContextAttachmentWidget extends Disposable {
3231
public readonly domNode: HTMLElement;
@@ -36,15 +35,14 @@ export class ImplicitContextAttachmentWidget extends Disposable {
3635
constructor(
3736
private readonly attachment: IChatRequestImplicitVariableEntry,
3837
private readonly resourceLabels: ResourceLabels,
39-
private readonly attachmentModel: ChatAttachmentModel,
4038
@IContextKeyService private readonly contextKeyService: IContextKeyService,
4139
@IContextMenuService private readonly contextMenuService: IContextMenuService,
4240
@ILabelService private readonly labelService: ILabelService,
4341
@IMenuService private readonly menuService: IMenuService,
4442
@IFileService private readonly fileService: IFileService,
4543
@ILanguageService private readonly languageService: ILanguageService,
4644
@IModelService private readonly modelService: IModelService,
47-
@IChatWidgetService private readonly chatWidgetService: IChatWidgetService,
45+
@IHoverService private readonly hoverService: IHoverService,
4846
) {
4947
super();
5048

@@ -56,10 +54,10 @@ export class ImplicitContextAttachmentWidget extends Disposable {
5654
dom.clearNode(this.domNode);
5755
this.renderDisposables.clear();
5856

59-
this.domNode.classList.add('disabled');
57+
this.domNode.classList.toggle('disabled', !this.attachment.enabled);
6058
const label = this.resourceLabels.create(this.domNode, { supportIcons: true });
6159
const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value!.uri;
62-
const range = undefined;
60+
const range = URI.isUri(this.attachment.value) || !this.attachment.isSelection ? undefined : this.attachment.value!.range;
6361

6462
const attachmentTypeName = (this.attachment.isPromptFile === false)
6563
? file.scheme === Schemas.vscodeNotebookCell ? localize('cell.lowercase', "cell") : localize('file.lowercase', "file")
@@ -68,7 +66,7 @@ export class ImplicitContextAttachmentWidget extends Disposable {
6866
const fileBasename = basename(file);
6967
const fileDirname = dirname(file);
7068
const friendlyName = `${fileBasename} ${fileDirname}`;
71-
const ariaLabel = localize('chat.fileAttachment', "Attached {0}, {1}", attachmentTypeName, friendlyName);
69+
const ariaLabel = range ? localize('chat.fileAttachmentWithRange', "Attached {0}, {1}, line {2} to line {3}", attachmentTypeName, friendlyName, range.startLineNumber, range.endLineNumber) : localize('chat.fileAttachment', "Attached {0}, {1}", attachmentTypeName, friendlyName);
7270

7371
const uriLabel = this.labelService.getUriLabel(file, { relative: true });
7472
const currentFile = localize('openEditor', "Suggested context (current file)");
@@ -88,17 +86,16 @@ export class ImplicitContextAttachmentWidget extends Disposable {
8886
this.domNode.ariaLabel = ariaLabel;
8987
this.domNode.tabIndex = 0;
9088

91-
this.renderDisposables.add(dom.addDisposableListener(this.domNode, dom.EventType.CLICK, e => {
92-
this.convertToRegularAttachment();
93-
}));
89+
const hintLabel = localize('hint.label.current', "Current {0}", attachmentTypeName);
90+
const hintElement = dom.append(this.domNode, dom.$('span.chat-implicit-hint', undefined, hintLabel));
91+
this._register(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), hintElement, title));
9492

95-
this.renderDisposables.add(dom.addDisposableListener(this.domNode, dom.EventType.KEY_DOWN, e => {
96-
const event = new StandardKeyboardEvent(e);
97-
if (event.equals(KeyCode.Enter) || event.equals(KeyCode.Space)) {
98-
e.preventDefault();
99-
e.stopPropagation();
100-
this.convertToRegularAttachment();
101-
}
93+
const buttonMsg = this.attachment.enabled ? localize('disable', "Disable current {0} context", attachmentTypeName) : localize('enable', "Enable current {0} context", attachmentTypeName);
94+
const toggleButton = this.renderDisposables.add(new Button(this.domNode, { supportIcons: true, title: buttonMsg }));
95+
toggleButton.icon = this.attachment.enabled ? Codicon.eye : Codicon.eyeClosed;
96+
this.renderDisposables.add(toggleButton.onDidClick((e) => {
97+
e.stopPropagation(); // prevent it from triggering the click handler on the parent immediately after rerendering
98+
this.attachment.enabled = !this.attachment.enabled;
10299
}));
103100

104101
// Context menu
@@ -121,14 +118,4 @@ export class ImplicitContextAttachmentWidget extends Disposable {
121118
});
122119
}));
123120
}
124-
125-
private convertToRegularAttachment(): void {
126-
if (!this.attachment.value) {
127-
return;
128-
}
129-
130-
const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value.uri;
131-
this.attachmentModel.addFile(file);
132-
this.chatWidgetService.lastFocusedWidget?.focusInput();
133-
}
134121
}

‎src/vs/workbench/contrib/chat/browser/chatInputPart.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { Disposable, DisposableStore, IDisposable, MutableDisposable, toDisposab
2525
import { ResourceSet } from '../../../../base/common/map.js';
2626
import { observableFromEvent } from '../../../../base/common/observable.js';
2727
import { isMacintosh } from '../../../../base/common/platform.js';
28-
import { isEqual } from '../../../../base/common/resources.js';
2928
import { assertType } from '../../../../base/common/types.js';
3029
import { URI } from '../../../../base/common/uri.js';
3130
import { IEditorConstructionOptions } from '../../../../editor/browser/config/editorConfiguration.js';
@@ -1306,6 +1305,11 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
13061305
this._indexOfLastOpenedContext = -1;
13071306
}
13081307

1308+
if (this.implicitContext?.value) {
1309+
const implicitPart = store.add(this.instantiationService.createInstance(ImplicitContextAttachmentWidget, this.implicitContext, this._contextResourceLabels));
1310+
container.appendChild(implicitPart.domNode);
1311+
}
1312+
13091313
this.promptFileAttached.set(this.hasPromptFileAttachments);
13101314
this.promptInstructionsAttachmentsPart.render(container);
13111315

@@ -1352,18 +1356,6 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
13521356
}));
13531357
}
13541358

1355-
const implicitUri = this.implicitContext?.value;
1356-
1357-
if (URI.isUri(implicitUri)) {
1358-
const currentlyAttached = attachments.some(
1359-
([, attachment]) => URI.isUri(attachment.value) && isEqual(attachment.value, implicitUri)
1360-
);
1361-
1362-
if (implicitUri && !currentlyAttached) {
1363-
const implicitPart = store.add(this.instantiationService.createInstance(ImplicitContextAttachmentWidget, this.implicitContext, this._contextResourceLabels, this._attachmentModel));
1364-
container.appendChild(implicitPart.domNode);
1365-
}
1366-
}
13671359

13681360
if (oldHeight !== this.attachmentsContainer.offsetHeight) {
13691361
this._onDidChangeHeight.fire();

‎src/vs/workbench/contrib/chat/browser/contrib/chatImplicitContext.ts

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { Emitter, Event } from '../../../../../base/common/event.js';
88
import { Disposable, DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js';
99
import { Schemas } from '../../../../../base/common/network.js';
1010
import { autorun } from '../../../../../base/common/observable.js';
11-
import { basename } from '../../../../../base/common/resources.js';
11+
import { basename, isEqual } from '../../../../../base/common/resources.js';
1212
import { URI } from '../../../../../base/common/uri.js';
1313
import { getCodeEditor, ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
1414
import { ICodeEditorService } from '../../../../../editor/browser/services/codeEditorService.js';
@@ -61,7 +61,9 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
6161
activeEditorDisposables.add(Event.debounce(
6262
Event.any(
6363
codeEditor.onDidChangeModel,
64-
codeEditor.onDidChangeModelLanguage),
64+
codeEditor.onDidChangeModelLanguage,
65+
codeEditor.onDidChangeCursorSelection,
66+
codeEditor.onDidScrollChange),
6567
() => undefined,
6668
500)(() => this.updateImplicitContext()));
6769
}
@@ -75,7 +77,9 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
7577
if (codeEditor && codeEditor.getModel()?.uri.scheme === Schemas.vscodeNotebookCell) {
7678
activeCellDisposables.add(Event.debounce(
7779
Event.any(
78-
codeEditor.onDidChangeModel),
80+
codeEditor.onDidChangeModel,
81+
codeEditor.onDidChangeCursorSelection,
82+
codeEditor.onDidScrollChange),
7983
() => undefined,
8084
500)(() => this.updateImplicitContext()));
8185
}
@@ -150,17 +154,57 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
150154
const cancelTokenSource = this._currentCancelTokenSource.value = new CancellationTokenSource();
151155
const codeEditor = this.findActiveCodeEditor();
152156
const model = codeEditor?.getModel();
157+
const selection = codeEditor?.getSelection();
153158
let newValue: Location | URI | undefined;
154-
const isSelection = false;
159+
let isSelection = false;
155160

156161
let languageId: string | undefined;
157162
if (model) {
158-
newValue = model.uri;
163+
languageId = model.getLanguageId();
164+
if (selection && !selection.isEmpty()) {
165+
newValue = { uri: model.uri, range: selection } satisfies Location;
166+
isSelection = true;
167+
} else {
168+
const visibleRanges = codeEditor?.getVisibleRanges();
169+
if (visibleRanges && visibleRanges.length > 0) {
170+
// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
171+
// Something like a Location with an array of Ranges?
172+
let range = visibleRanges[0];
173+
visibleRanges.slice(1).forEach(r => {
174+
range = range.plusRange(r);
175+
});
176+
newValue = { uri: model.uri, range } satisfies Location;
177+
} else {
178+
newValue = model.uri;
179+
}
180+
}
159181
}
160182

161183
const notebookEditor = this.findActiveNotebookEditor();
162184
if (notebookEditor) {
163-
newValue = notebookEditor.textModel?.uri;
185+
const activeCell = notebookEditor.getActiveCell();
186+
if (activeCell) {
187+
const codeEditor = this.codeEditorService.getActiveCodeEditor();
188+
const selection = codeEditor?.getSelection();
189+
const visibleRanges = codeEditor?.getVisibleRanges() || [];
190+
newValue = activeCell.uri;
191+
if (isEqual(codeEditor?.getModel()?.uri, activeCell.uri)) {
192+
if (selection && !selection.isEmpty()) {
193+
newValue = { uri: activeCell.uri, range: selection } satisfies Location;
194+
isSelection = true;
195+
} else if (visibleRanges.length > 0) {
196+
// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
197+
// Something like a Location with an array of Ranges?
198+
let range = visibleRanges[0];
199+
visibleRanges.slice(1).forEach(r => {
200+
range = range.plusRange(r);
201+
});
202+
newValue = { uri: activeCell.uri, range } satisfies Location;
203+
}
204+
}
205+
} else {
206+
newValue = notebookEditor.textModel?.uri;
207+
}
164208
}
165209

166210
const uri = newValue instanceof URI ? newValue : newValue?.uri;
@@ -269,7 +313,7 @@ export class ChatImplicitContext extends Disposable implements IChatRequestImpli
269313
return this._value;
270314
}
271315

272-
private _enabled = false;
316+
private _enabled = true;
273317
get enabled() {
274318
return this._enabled;
275319
}

‎src/vs/workbench/contrib/chat/browser/media/chat.css

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,12 +1366,6 @@ have to be updated for changes to the rules above, or to support more deeply nes
13661366
border-style: dashed;
13671367
}
13681368

1369-
.interactive-session .chat-attached-context .chat-attached-context-attachment.implicit.disabled:hover {
1370-
cursor: pointer;
1371-
border-style: solid;
1372-
background-color: var(--vscode-toolbar-hoverBackground);
1373-
}
1374-
13751369
.interactive-session .chat-attached-context .chat-attached-context-attachment.implicit.disabled:focus {
13761370
outline: none;
13771371
border-color: var(--vscode-focusBorder);

0 commit comments

Comments
 (0)