Make "attach" only a QuickAccess thing, not a QuickPick thing

src/vs/editor/contrib/quickAccess/browser/gotoSymbolQuickAccess.ts

@@ -165,14 +165,6 @@ export abstract class AbstractGotoSymbolQuickAccessProvider extends AbstractEdit
 			}
 		}));
 
-		// Attach the active symbol as context
-		disposables.add(picker.onDidAttach(() => {
-			const [item] = picker.activeItems;
-			if (typeof item?.attach === 'function') {
-				item.attach();
-			}
-		}));
-
 		// Resolve symbols from document once and reuse this
 		// request for all filtering and typing then on
 		const symbolsPromise = this.getDocumentSymbols(model, token);

src/vs/platform/quickinput/browser/pickerQuickAccess.ts

@@ -6,7 +6,7 @@
 import { timeout } from '../../../base/common/async.js';
 import { CancellationToken, CancellationTokenSource } from '../../../base/common/cancellation.js';
 import { Disposable, DisposableStore, IDisposable, MutableDisposable } from '../../../base/common/lifecycle.js';
-import { IKeyMods, IQuickPickDidAcceptEvent, IQuickPickSeparator, IQuickPick, IQuickPickItem, IQuickInputButton } from '../common/quickInput.js';
+import { IKeyMods, IQuickPickDidAcceptEvent, IQuickPickSeparator, IQuickPick, IQuickPickItem, IQuickInputButton, isKeyModified } from '../common/quickInput.js';
 import { IQuickAccessProvider, IQuickAccessProviderRunOptions } from '../common/quickAccess.js';
 import { isFunction } from '../../../base/common/types.js';
 
@@ -51,18 +51,22 @@ export interface IPickerQuickAccessItem extends IQuickPickItem {
 	 * @param buttonIndex index of the button of the item that
 	 * was clicked.
 	 *
-	 * @param the state of modifier keys when the button was triggered.
+	 * @param keyMods the state of modifier keys when the button was triggered.
 	 *
 	 * @returns a value that indicates what should happen after the trigger
 	 * which can be a `Promise` for long running operations.
 	 */
 	trigger?(buttonIndex: number, keyMods: IKeyMods): TriggerAction | Promise<TriggerAction>;
 
 	/**
-	 * A method that will be executed when the pick item should be attached
-	 * as context, e.g. to a chat conversation.
+	 * When set, this will be invoked instead of `accept` if modifier keys are held down.
+	 * This is useful for actions like "attach to context" where you want to keep the picker
+	 * open and allow multiple picks.
+	 *
+	 * @param keyMods the state of modifier keys when the item was accepted.
+	 * @param event the underlying event that caused this to trigger.
 	 */
-	attach?(): void;
+	attach?(keyMods: IKeyMods, event: IQuickPickDidAcceptEvent): void;
 }
 
 export interface IPickerQuickAccessSeparator extends IQuickPickSeparator {
@@ -73,7 +77,7 @@ export interface IPickerQuickAccessSeparator extends IQuickPickSeparator {
 	 * @param buttonIndex index of the button of the item that
 	 * was clicked.
 	 *
-	 * @param the state of modifier keys when the button was triggered.
+	 * @param keyMods the state of modifier keys when the button was triggered.
 	 *
 	 * @returns a value that indicates what should happen after the trigger
 	 * which can be a `Promise` for long running operations.
@@ -343,6 +347,11 @@ export abstract class PickerQuickAccessProvider<T extends IPickerQuickAccessItem
 
 			const [item] = picker.selectedItems;
 			if (typeof item?.accept === 'function') {
+				const isAttachAction = isKeyModified(picker.keyMods) && !!item.attach;
+				if (isAttachAction) {
+					item.attach!(picker.keyMods, event);
+					return;
+				}
 				if (!event.inBackground) {
 					picker.hide(); // hide picker unless we accept in background
 				}
@@ -351,14 +360,6 @@ export abstract class PickerQuickAccessProvider<T extends IPickerQuickAccessItem
 			}
 		}));
 
-		// Attach the active pick item as context
-		disposables.add(picker.onDidAttach(() => {
-			const [item] = picker.activeItems;
-			if (typeof item?.attach === 'function') {
-				item.attach();
-			}
-		}));
-
 		const buttonTrigger = async (button: IQuickInputButton, item: T | IPickerQuickAccessSeparator) => {
 			if (typeof item.trigger !== 'function') {
 				return;

src/vs/platform/quickinput/browser/quickInput.ts

@@ -532,7 +532,6 @@ export class QuickPick<T extends IQuickPickItem, O extends { useSeparators: bool
 	private readonly onWillAcceptEmitter = this._register(new Emitter<IQuickPickWillAcceptEvent>());
 	private readonly onDidAcceptEmitter = this._register(new Emitter<IQuickPickDidAcceptEvent>());
 	private readonly onDidCustomEmitter = this._register(new Emitter<void>());
-	private readonly onDidAttachEmitter = this._register(new Emitter<void>());
 	private _items: O extends { useSeparators: true } ? Array<T | IQuickPickSeparator> : Array<T> = [];
 	private itemsUpdated = false;
 	private _canSelectMany = false;
@@ -644,8 +643,6 @@ export class QuickPick<T extends IQuickPickItem, O extends { useSeparators: bool
 
 	onDidCustom = this.onDidCustomEmitter.event;
 
-	onDidAttach = this.onDidAttachEmitter.event;
-
 	get items() {
 		return this._items;
 	}
@@ -1180,10 +1177,6 @@ export class QuickPick<T extends IQuickPickItem, O extends { useSeparators: bool
 		}
 		this.handleAccept(inBackground ?? false);
 	}
-
-	attach(): void {
-		this.onDidAttachEmitter.fire();
-	}
 }
 
 export class InputBox extends QuickInput implements IInputBox {

src/vs/platform/quickinput/browser/quickInputActions.ts

@@ -44,7 +44,7 @@ function registerQuickPickCommandAndKeybindingRule(rule: PartialExcept<ICommandA
 const ctrlKeyMod = isMacintosh ? KeyMod.WinCtrl : KeyMod.CtrlCmd;
 
 // This function will generate all the combinations of keybindings for the given primary keybinding
-function getSecondary(primary: number, secondary: number[], options: { withAltMod?: boolean; withCtrlMod?: boolean; withCmdMod?: boolean } = {}): number[] {
+function getSecondary(primary: number, secondary: number[], options: { withAltMod?: boolean; withCtrlMod?: boolean; withCmdMod?: boolean; withShiftMod?: boolean } = {}): number[] {
 	if (options.withAltMod) {
 		secondary.push(KeyMod.Alt + primary);
 	}
@@ -54,6 +54,18 @@ function getSecondary(primary: number, secondary: number[], options: { withAltMo
 			secondary.push(KeyMod.Alt + ctrlKeyMod + primary);
 		}
 	}
+	if (options.withShiftMod) {
+		secondary.push(KeyMod.Shift + primary);
+		if (options.withAltMod) {
+			secondary.push(KeyMod.Alt + KeyMod.Shift + primary);
+		}
+		if (options.withCtrlMod) {
+			secondary.push(ctrlKeyMod + KeyMod.Shift + primary);
+			if (options.withAltMod) {
+				secondary.push(KeyMod.Alt + ctrlKeyMod + KeyMod.Shift + primary);
+			}
+		}
+	}
 
 	if (options.withCmdMod && isMacintosh) {
 		secondary.push(KeyMod.CtrlCmd + primary);
@@ -213,7 +225,7 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
 		const currentQuickPick = accessor.get(IQuickInputService).currentQuickInput as IQuickPick<any> | IQuickTree<any> | IInputBox;
 		currentQuickPick?.accept();
 	},
-	secondary: getSecondary(KeyCode.Enter, [], { withAltMod: true, withCtrlMod: true, withCmdMod: true })
+	secondary: getSecondary(KeyCode.Enter, [], { withAltMod: true, withCtrlMod: true, withCmdMod: true, withShiftMod: true })
 });
 
 registerQuickPickCommandAndKeybindingRule(
@@ -239,26 +251,6 @@ registerQuickPickCommandAndKeybindingRule(
 
 //#endregion
 
-//#region Attach
-
-registerQuickPickCommandAndKeybindingRule(
-	{
-		id: 'quickInput.attach',
-		when: ContextKeyExpr.and(
-			inQuickInputContext,
-			ContextKeyExpr.equals(quickInputTypeContextKeyValue, QuickInputType.QuickPick),
-		),
-		primary: KeyMod.Shift | KeyCode.Enter,
-		weight: KeybindingWeight.WorkbenchContrib + 100,
-		handler: (accessor) => {
-			const currentQuickPick = accessor.get(IQuickInputService).currentQuickInput as IQuickPick<any>;
-			currentQuickPick?.attach();
-		},
-	},
-);
-
-//#endregion
-
 //#region Hide
 
 registerQuickInputCommandAndKeybindingRule(

src/vs/platform/quickinput/browser/quickInputController.ts

@@ -60,7 +60,7 @@ export class QuickInputController extends Disposable {
 	private readonly onDidAcceptEmitter = this._register(new Emitter<void>());
 	private readonly onDidCustomEmitter = this._register(new Emitter<void>());
 	private readonly onDidTriggerButtonEmitter = this._register(new Emitter<IQuickInputButton>());
-	private keyMods: Writeable<IKeyMods> = { ctrlCmd: false, alt: false };
+	private keyMods: Writeable<IKeyMods> = { ctrlCmd: false, alt: false, shift: false };
 
 	private controller: IQuickInput | null = null;
 	get currentQuickInput() { return this.controller ?? undefined; }
@@ -120,6 +120,7 @@ export class QuickInputController extends Disposable {
 		const listener = (e: KeyboardEvent | MouseEvent) => {
 			this.keyMods.ctrlCmd = e.ctrlKey || e.metaKey;
 			this.keyMods.alt = e.altKey;
+			this.keyMods.shift = e.shiftKey;
 		};
 
 		for (const event of [dom.EventType.KEY_DOWN, dom.EventType.KEY_UP, dom.EventType.MOUSE_DOWN]) {
@@ -837,13 +838,14 @@ export class QuickInputController extends Disposable {
 		}
 	}
 
-	async accept(keyMods: IKeyMods = { alt: false, ctrlCmd: false }) {
+	async accept(keyMods: IKeyMods = { alt: false, ctrlCmd: false, shift: false }) {
 		// When accepting the item programmatically, it is important that
 		// we update `keyMods` either from the provided set or unset it
 		// because the accept did not happen from mouse or keyboard
 		// interaction on the list itself
 		this.keyMods.alt = keyMods.alt;
 		this.keyMods.ctrlCmd = keyMods.ctrlCmd;
+		this.keyMods.shift = keyMods.shift;
 
 		this.onDidAcceptEmitter.fire();
 	}

src/vs/platform/quickinput/common/quickInput.ts

@@ -115,9 +115,14 @@ export interface IQuickPickSeparator {
 export interface IKeyMods {
 	readonly ctrlCmd: boolean;
 	readonly alt: boolean;
+	readonly shift: boolean;
 }
 
-export const NO_KEY_MODS: IKeyMods = { ctrlCmd: false, alt: false };
+export function isKeyModified(keyMods: IKeyMods): boolean {
+	return keyMods.ctrlCmd || keyMods.alt || keyMods.shift;
+}
+
+export const NO_KEY_MODS: IKeyMods = { ctrlCmd: false, alt: false, shift: false };
 
 export interface IQuickNavigateConfiguration {
 	keybindings: readonly ResolvedKeybinding[];
@@ -713,16 +718,6 @@ export interface IQuickPick<T extends IQuickPickItem, O extends { useSeparators:
 	 * @param inBackground Whether you are accepting an item in the background and keeping the picker open.
 	 */
 	accept(inBackground?: boolean): void;
-
-	/**
-	 * An event that is fired when an item should be attached as context.
-	 */
-	readonly onDidAttach: Event<void>;
-
-	/**
-	 * Programmatically triggers the attach action for the active item.
-	 */
-	attach(): void;
 }
 
 /**

src/vs/platform/quickinput/test/browser/quickinput.test.ts

@@ -19,7 +19,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/c
 import { toDisposable } from '../../../../base/common/lifecycle.js';
 import { mainWindow } from '../../../../base/browser/window.js';
 import { QuickPick } from '../../browser/quickInput.js';
-import { IQuickPickItem, ItemActivation } from '../../common/quickInput.js';
+import { IQuickPickItem, ItemActivation, isKeyModified, NO_KEY_MODS } from '../../common/quickInput.js';
 import { TestInstantiationService } from '../../../instantiation/test/common/instantiationServiceMock.js';
 import { IThemeService } from '../../../theme/common/themeService.js';
 import { IConfigurationService } from '../../../configuration/common/configuration.js';
@@ -278,4 +278,16 @@ suite('QuickInput', () => { // https://github.com/microsoft/vscode/issues/147543
 		assert.strictEqual(activeItemsFromEvent.length, 0);
 		assert.strictEqual(quickpick.activeItems.length, 0);
 	});
+
+	test('isKeyModified - returns false when no modifiers are pressed', () => {
+		assert.strictEqual(isKeyModified(NO_KEY_MODS), false);
+		assert.strictEqual(isKeyModified({ ctrlCmd: false, alt: false, shift: false }), false);
+	});
+
+	test('isKeyModified - returns true when any modifier is pressed', () => {
+		assert.strictEqual(isKeyModified({ ctrlCmd: true, alt: false, shift: false }), true);
+		assert.strictEqual(isKeyModified({ ctrlCmd: false, alt: true, shift: false }), true);
+		assert.strictEqual(isKeyModified({ ctrlCmd: false, alt: false, shift: true }), true);
+		assert.strictEqual(isKeyModified({ ctrlCmd: true, alt: true, shift: true }), true);
+	});
 });

src/vs/workbench/browser/actions/quickAccessActions.ts

@@ -56,7 +56,7 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
 	primary: 0,
 	handler: accessor => {
 		const quickInputService = accessor.get(IQuickInputService);
-		return quickInputService.accept({ ctrlCmd: true, alt: false });
+		return quickInputService.accept({ ctrlCmd: true, alt: false, shift: false });
 	}
 });
 

src/vs/workbench/contrib/search/browser/anythingQuickAccess.ts

@@ -1086,11 +1086,18 @@ export class AnythingQuickAccessProvider extends PickerQuickAccessProvider<IAnyt
 				return TriggerAction.NO_ACTION;
 			},
 			accept: (keyMods, event) => this.openAnything(resourceOrEditor, { keyMods, range: this.pickState.lastRange, preserveFocus: event.inBackground, forcePinned: event.inBackground }),
-			attach: () => {
-				const widget = this.chatWidgetService.lastFocusedWidget;
-				if (widget && resource) {
-					widget.attachmentModel.addContext(widget.attachmentModel.asFileVariableEntry(resource));
+			attach: (keyMods, event) => {
+				// Only support adding context to chat when shift is pressed
+				if (keyMods.shift) {
+					const widget = this.chatWidgetService.lastFocusedWidget;
+					if (widget && resource) {
+						widget.attachmentModel.addContext(widget.attachmentModel.asFileVariableEntry(resource));
+					}
+					return;
 				}
+
+				// Fallback to accept behavior.
+				this.openAnything(resourceOrEditor, { keyMods, range: this.pickState.lastRange, preserveFocus: event.inBackground, forcePinned: event.inBackground });
 			}
 		};
 	}

src/vs/workbench/contrib/search/browser/symbolsQuickAccess.ts

@@ -214,18 +214,25 @@ export class SymbolsQuickAccessProvider extends PickerQuickAccessProvider<ISymbo
 					return TriggerAction.CLOSE_PICKER;
 				},
 				accept: async (keyMods, event) => this.openSymbol(provider, symbol, token, { keyMods, preserveFocus: event.inBackground, forcePinned: event.inBackground }),
-				attach: () => {
-					const widget = this.chatWidgetService.lastFocusedWidget;
-					if (widget) {
-						const entry: ISymbolVariableEntry = {
-							kind: 'symbol',
-							id: JSON.stringify({ uri: symbolUri.toString(), range: symbol.location.range }),
-							name: symbol.name,
-							value: symbol.location,
-							symbolKind: symbol.kind,
-						};
-						widget.attachmentModel.addContext(entry);
+				attach: (keyMods, event) => {
+					// Only support adding context to chat when shift is pressed
+					if (keyMods.shift) {
+						const widget = this.chatWidgetService.lastFocusedWidget;
+						if (widget) {
+							const entry: ISymbolVariableEntry = {
+								kind: 'symbol',
+								id: JSON.stringify({ uri: symbolUri.toString(), range: symbol.location.range }),
+								name: symbol.name,
+								value: symbol.location,
+								symbolKind: symbol.kind,
+							};
+							widget.attachmentModel.addContext(entry);
+						}
+						return;
 					}
+
+					// Fallback to accept behavior.
+					this.openSymbol(provider, symbol, token, { keyMods, preserveFocus: event.inBackground, forcePinned: event.inBackground });
 				},
 			});
 

src/vs/workbench/contrib/terminal/test/browser/terminalProfileService.integrationTest.ts

@@ -363,14 +363,14 @@ suite('TerminalProfileService', () => {
 
 		test('createInstance', async () => {
 			mockTerminalProfileService.setDefaultProfileName(powershellProfile.profileName);
-			const pick = { ...powershellPick, keyMods: { alt: true, ctrlCmd: false } };
+			const pick = { ...powershellPick, keyMods: { alt: true, ctrlCmd: false, shift: false } };
 			quickInputService.setPick(pick);
 			const result = await terminalProfileQuickpick.showAndGetResult('createInstance');
-			deepStrictEqual(result, { config: powershellProfile, keyMods: { alt: true, ctrlCmd: false } });
+			deepStrictEqual(result, { config: powershellProfile, keyMods: { alt: true, ctrlCmd: false, shift: false } });
 		});
 
 		test('createInstance with contributed', async () => {
-			const pick = { ...jsdebugPick, keyMods: { alt: true, ctrlCmd: false } };
+			const pick = { ...jsdebugPick, keyMods: { alt: true, ctrlCmd: false, shift: true } };
 			quickInputService.setPick(pick);
 			const result = await terminalProfileQuickpick.showAndGetResult('createInstance');
 			const expected = {
@@ -380,7 +380,7 @@ suite('TerminalProfileService', () => {
 					options: { color: undefined, icon: 'debug' },
 					title: jsdebugProfile.title,
 				},
-				keyMods: { alt: true, ctrlCmd: false }
+				keyMods: { alt: true, ctrlCmd: false, shift: true }
 			};
 			deepStrictEqual(result, expected);
 		});