revert implicit context to old implicit context flow (microsoft#251637)

Author: justschen

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

@@ -4,9 +4,10 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as dom from '../../../../../base/browser/dom.js';
-import { StandardKeyboardEvent } from '../../../../../base/browser/keyboardEvent.js';
 import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js';
-import { KeyCode } from '../../../../../base/common/keyCodes.js';
+import { Button } from '../../../../../base/browser/ui/button/button.js';
+import { getDefaultHoverDelegate } from '../../../../../base/browser/ui/hover/hoverDelegateFactory.js';
+import { Codicon } from '../../../../../base/common/codicons.js';
 import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js';
 import { Schemas } from '../../../../../base/common/network.js';
 import { basename, dirname } from '../../../../../base/common/resources.js';
@@ -19,12 +20,11 @@ import { IMenuService, MenuId } from '../../../../../platform/actions/common/act
 import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
 import { IContextMenuService } from '../../../../../platform/contextview/browser/contextView.js';
 import { FileKind, IFileService } from '../../../../../platform/files/common/files.js';
+import { IHoverService } from '../../../../../platform/hover/browser/hover.js';
 import { ILabelService } from '../../../../../platform/label/common/label.js';
 import { ResourceLabels } from '../../../../browser/labels.js';
 import { ResourceContextKey } from '../../../../common/contextkeys.js';
 import { IChatRequestImplicitVariableEntry } from '../../common/chatVariableEntries.js';
-import { IChatWidgetService } from '../chat.js';
-import { ChatAttachmentModel } from '../chatAttachmentModel.js';
 
 export class ImplicitContextAttachmentWidget extends Disposable {
 	public readonly domNode: HTMLElement;
@@ -34,15 +34,14 @@ export class ImplicitContextAttachmentWidget extends Disposable {
 	constructor(
 		private readonly attachment: IChatRequestImplicitVariableEntry,
 		private readonly resourceLabels: ResourceLabels,
-		private readonly attachmentModel: ChatAttachmentModel,
 		@IContextKeyService private readonly contextKeyService: IContextKeyService,
 		@IContextMenuService private readonly contextMenuService: IContextMenuService,
 		@ILabelService private readonly labelService: ILabelService,
 		@IMenuService private readonly menuService: IMenuService,
 		@IFileService private readonly fileService: IFileService,
 		@ILanguageService private readonly languageService: ILanguageService,
 		@IModelService private readonly modelService: IModelService,
-		@IChatWidgetService private readonly chatWidgetService: IChatWidgetService,
+		@IHoverService private readonly hoverService: IHoverService,
 	) {
 		super();
 
@@ -54,17 +53,17 @@ export class ImplicitContextAttachmentWidget extends Disposable {
 		dom.clearNode(this.domNode);
 		this.renderDisposables.clear();
 
-		this.domNode.classList.add('disabled');
+		this.domNode.classList.toggle('disabled', !this.attachment.enabled);
 		const label = this.resourceLabels.create(this.domNode, { supportIcons: true });
 		const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value!.uri;
-		const range = undefined;
+		const range = URI.isUri(this.attachment.value) || !this.attachment.isSelection ? undefined : this.attachment.value!.range;
 
 		const attachmentTypeName = file.scheme === Schemas.vscodeNotebookCell ? localize('cell.lowercase', "cell") : localize('file.lowercase', "file");
 
 		const fileBasename = basename(file);
 		const fileDirname = dirname(file);
 		const friendlyName = `${fileBasename} ${fileDirname}`;
-		const ariaLabel = localize('chat.fileAttachment', "Attached {0}, {1}", attachmentTypeName, friendlyName);
+		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);
 
 		const uriLabel = this.labelService.getUriLabel(file, { relative: true });
 		const currentFile = localize('openEditor', "Suggested context (current file)");
@@ -79,17 +78,16 @@ export class ImplicitContextAttachmentWidget extends Disposable {
 		this.domNode.ariaLabel = ariaLabel;
 		this.domNode.tabIndex = 0;
 
-		this.renderDisposables.add(dom.addDisposableListener(this.domNode, dom.EventType.CLICK, e => {
-			this.convertToRegularAttachment();
-		}));
+		const hintLabel = localize('hint.label.current', "Current {0}", attachmentTypeName);
+		const hintElement = dom.append(this.domNode, dom.$('span.chat-implicit-hint', undefined, hintLabel));
+		this._register(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), hintElement, title));
 
-		this.renderDisposables.add(dom.addDisposableListener(this.domNode, dom.EventType.KEY_DOWN, e => {
-			const event = new StandardKeyboardEvent(e);
-			if (event.equals(KeyCode.Enter) || event.equals(KeyCode.Space)) {
-				e.preventDefault();
-				e.stopPropagation();
-				this.convertToRegularAttachment();
-			}
+		const buttonMsg = this.attachment.enabled ? localize('disable', "Disable current {0} context", attachmentTypeName) : localize('enable', "Enable current {0} context", attachmentTypeName);
+		const toggleButton = this.renderDisposables.add(new Button(this.domNode, { supportIcons: true, title: buttonMsg }));
+		toggleButton.icon = this.attachment.enabled ? Codicon.eye : Codicon.eyeClosed;
+		this.renderDisposables.add(toggleButton.onDidClick((e) => {
+			e.stopPropagation(); // prevent it from triggering the click handler on the parent immediately after rerendering
+			this.attachment.enabled = !this.attachment.enabled;
 		}));
 
 		// Context menu
@@ -112,14 +110,4 @@ export class ImplicitContextAttachmentWidget extends Disposable {
 			});
 		}));
 	}
-
-	private convertToRegularAttachment(): void {
-		if (!this.attachment.value) {
-			return;
-		}
-
-		const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value.uri;
-		this.attachmentModel.addFile(file);
-		this.chatWidgetService.lastFocusedWidget?.focusInput();
-	}
 }

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

@@ -25,7 +25,6 @@ import { ResourceSet } from '../../../../base/common/map.js';
 import { observableFromEvent } from '../../../../base/common/observable.js';
 import { isMacintosh } from '../../../../base/common/platform.js';
 import { ScrollbarVisibility } from '../../../../base/common/scrollable.js';
-import { isEqual } from '../../../../base/common/resources.js';
 import { assertType } from '../../../../base/common/types.js';
 import { URI } from '../../../../base/common/uri.js';
 import { Schemas } from '../../../../base/common/network.js';
@@ -1250,6 +1249,11 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
 			this._indexOfLastOpenedContext = -1;
 		}
 
+		if (this.implicitContext?.value) {
+			const implicitPart = store.add(this.instantiationService.createInstance(ImplicitContextAttachmentWidget, this.implicitContext, this._contextResourceLabels));
+			container.appendChild(implicitPart.domNode);
+		}
+
 		this.promptFileAttached.set(this.hasPromptFileAttachments);
 
 		for (const [index, attachment] of attachments) {
@@ -1297,18 +1301,6 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge
 			}));
 		}
 
-		const implicitUri = this.implicitContext?.value;
-
-		if (URI.isUri(implicitUri)) {
-			const currentlyAttached = attachments.some(
-				([, attachment]) => URI.isUri(attachment.value) && isEqual(attachment.value, implicitUri)
-			);
-
-			if (implicitUri && !currentlyAttached) {
-				const implicitPart = store.add(this.instantiationService.createInstance(ImplicitContextAttachmentWidget, this.implicitContext, this._contextResourceLabels, this._attachmentModel));
-				container.appendChild(implicitPart.domNode);
-			}
-		}
 
 		if (oldHeight !== this.attachmentsContainer.offsetHeight) {
 			this._onDidChangeHeight.fire();

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

@@ -8,7 +8,7 @@ import { Emitter, Event } from '../../../../../base/common/event.js';
 import { Disposable, DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js';
 import { Schemas } from '../../../../../base/common/network.js';
 import { autorun } from '../../../../../base/common/observable.js';
-import { basename } from '../../../../../base/common/resources.js';
+import { basename, isEqual } from '../../../../../base/common/resources.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { getCodeEditor, ICodeEditor } from '../../../../../editor/browser/editorBrowser.js';
 import { ICodeEditorService } from '../../../../../editor/browser/services/codeEditorService.js';
@@ -57,7 +57,9 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
 					activeEditorDisposables.add(Event.debounce(
 						Event.any(
 							codeEditor.onDidChangeModel,
-							codeEditor.onDidChangeModelLanguage),
+							codeEditor.onDidChangeModelLanguage,
+							codeEditor.onDidChangeCursorSelection,
+							codeEditor.onDidScrollChange),
 						() => undefined,
 						500)(() => this.updateImplicitContext()));
 				}
@@ -71,7 +73,9 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
 						if (codeEditor && codeEditor.getModel()?.uri.scheme === Schemas.vscodeNotebookCell) {
 							activeCellDisposables.add(Event.debounce(
 								Event.any(
-									codeEditor.onDidChangeModel),
+									codeEditor.onDidChangeModel,
+									codeEditor.onDidChangeCursorSelection,
+									codeEditor.onDidScrollChange),
 								() => undefined,
 								500)(() => this.updateImplicitContext()));
 						}
@@ -146,18 +150,57 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
 		const cancelTokenSource = this._currentCancelTokenSource.value = new CancellationTokenSource();
 		const codeEditor = this.findActiveCodeEditor();
 		const model = codeEditor?.getModel();
+		const selection = codeEditor?.getSelection();
 		let newValue: Location | URI | undefined;
-		const isSelection = false;
+		let isSelection = false;
 
 		let languageId: string | undefined;
 		if (model) {
-			newValue = model.uri;
 			languageId = model.getLanguageId();
+			if (selection && !selection.isEmpty()) {
+				newValue = { uri: model.uri, range: selection } satisfies Location;
+				isSelection = true;
+			} else {
+				const visibleRanges = codeEditor?.getVisibleRanges();
+				if (visibleRanges && visibleRanges.length > 0) {
+					// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
+					// Something like a Location with an array of Ranges?
+					let range = visibleRanges[0];
+					visibleRanges.slice(1).forEach(r => {
+						range = range.plusRange(r);
+					});
+					newValue = { uri: model.uri, range } satisfies Location;
+				} else {
+					newValue = model.uri;
+				}
+			}
 		}
 
 		const notebookEditor = this.findActiveNotebookEditor();
 		if (notebookEditor) {
-			newValue = notebookEditor.textModel?.uri;
+			const activeCell = notebookEditor.getActiveCell();
+			if (activeCell) {
+				const codeEditor = this.codeEditorService.getActiveCodeEditor();
+				const selection = codeEditor?.getSelection();
+				const visibleRanges = codeEditor?.getVisibleRanges() || [];
+				newValue = activeCell.uri;
+				if (isEqual(codeEditor?.getModel()?.uri, activeCell.uri)) {
+					if (selection && !selection.isEmpty()) {
+						newValue = { uri: activeCell.uri, range: selection } satisfies Location;
+						isSelection = true;
+					} else if (visibleRanges.length > 0) {
+						// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
+						// Something like a Location with an array of Ranges?
+						let range = visibleRanges[0];
+						visibleRanges.slice(1).forEach(r => {
+							range = range.plusRange(r);
+						});
+						newValue = { uri: activeCell.uri, range } satisfies Location;
+					}
+				}
+			} else {
+				newValue = notebookEditor.textModel?.uri;
+			}
 		}
 
 		const uri = newValue instanceof URI ? newValue : newValue?.uri;
@@ -243,7 +286,7 @@ export class ChatImplicitContext extends Disposable implements IChatRequestImpli
 		return this._value;
 	}
 
-	private _enabled = false;
+	private _enabled = true;
 	get enabled() {
 		return this._enabled;
 	}

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

@@ -1319,12 +1319,6 @@ have to be updated for changes to the rules above, or to support more deeply nes
 	border-style: dashed;
 }
 
-.interactive-session .chat-attached-context .chat-attached-context-attachment.implicit.disabled:hover {
-	cursor: pointer;
-	border-style: solid;
-	background-color: var(--vscode-toolbar-hoverBackground);
-}
-
 .interactive-session .chat-attached-context .chat-attached-context-attachment.implicit.disabled:focus {
 	outline: none;
 	border-color: var(--vscode-focusBorder);

src/vs/workbench/contrib/chat/common/chatVariableEntries.ts

@@ -67,7 +67,7 @@ export interface IChatRequestImplicitVariableEntry extends IBaseChatRequestVaria
 	readonly isFile: true;
 	readonly value: URI | Location | undefined;
 	readonly isSelection: boolean;
-	readonly enabled: boolean;
+	enabled: boolean;
 }
 
 export interface IChatRequestPasteVariableEntry extends IBaseChatRequestVariableEntry {