Use # for chat variables (#194063)

* Use # for chat variables

* Fix test

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

@@ -26,7 +26,7 @@ import { ChatInputPart } from 'vs/workbench/contrib/chat/browser/chatInputPart';
 import { ChatWidget } from 'vs/workbench/contrib/chat/browser/chatWidget';
 import { IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
 import { chatSlashCommandBackground, chatSlashCommandForeground } from 'vs/workbench/contrib/chat/common/chatColors';
-import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart } from 'vs/workbench/contrib/chat/common/chatParserTypes';
+import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart, chatVariableLeader } from 'vs/workbench/contrib/chat/common/chatParserTypes';
 import { ChatRequestParser } from 'vs/workbench/contrib/chat/common/chatRequestParser';
 import { IChatService, ISlashCommand } from 'vs/workbench/contrib/chat/common/chatService';
 import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables';
@@ -496,7 +496,7 @@ function computeCompletionRanges(model: ITextModel, position: Position, reg: Reg
 
 class VariableCompletions extends Disposable {
 
-	private static readonly VariableNameDef = /@\w*/g; // MUST be using `g`-flag
+	private static readonly VariableNameDef = new RegExp(`${chatVariableLeader}\\w*`, 'g'); // MUST be using `g`-flag
 
 	constructor(
 		@ILanguageFeaturesService private readonly languageFeaturesService: ILanguageFeaturesService,
@@ -508,7 +508,7 @@ class VariableCompletions extends Disposable {
 
 		this._register(this.languageFeaturesService.completionProvider.register({ scheme: ChatInputPart.INPUT_SCHEME, hasAccessToAllModels: true }, {
 			_debugDisplayName: 'chatVariables',
-			triggerCharacters: ['@'],
+			triggerCharacters: [chatVariableLeader],
 			provideCompletionItems: async (model: ITextModel, position: Position, _context: CompletionContext, _token: CancellationToken) => {
 
 				const widget = this.chatWidgetService.getWidgetByInputUri(model.uri);
@@ -527,19 +527,19 @@ class VariableCompletions extends Disposable {
 				// TODO@roblourens work out a real API for this- maybe it can be part of the two-step flow that @file will probably use
 				const historyVariablesEnabled = this.configurationService.getValue('chat.experimental.historyVariables');
 				const historyItems = historyVariablesEnabled ? history.map((h, i): CompletionItem => ({
-					label: `@response:${i + 1}`,
+					label: `${chatVariableLeader}response:${i + 1}`,
 					detail: h.response.asString(),
-					insertText: `@response:${String(i + 1).padStart(String(history.length).length, '0')} `,
+					insertText: `${chatVariableLeader}response:${String(i + 1).padStart(String(history.length).length, '0')} `,
 					kind: CompletionItemKind.Text,
 					range,
 				})) : [];
 
 				const variableItems = Array.from(this.chatVariablesService.getVariables()).map(v => {
-					const withAt = `@${v.name}`;
+					const withLeader = `${chatVariableLeader}${v.name}`;
 					return <CompletionItem>{
-						label: withAt,
+						label: withLeader,
 						range,
-						insertText: withAt + ' ',
+						insertText: withLeader + ' ',
 						detail: v.description,
 						kind: CompletionItemKind.Text, // The icons are disabled here anyway,
 					};

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

@@ -27,6 +27,8 @@ export class ChatRequestTextPart implements IParsedChatRequestPart {
 	constructor(readonly range: OffsetRange, readonly editorRange: IRange, readonly text: string) { }
 }
 
+export const chatVariableLeader = '#'; // warning, this also shows up in a regex in the parser
+
 /**
  * An invocation of a static variable that can be resolved by the variable service
  */
@@ -35,7 +37,7 @@ export class ChatRequestVariablePart implements IParsedChatRequestPart {
 
 	get text(): string {
 		const argPart = this.variableArg ? `:${this.variableArg}` : '';
-		return `@${this.variableName}${argPart}`;
+		return `${chatVariableLeader}${this.variableName}${argPart}`;
 	}
 }
 

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

@@ -8,11 +8,12 @@ import { OffsetRange } from 'vs/editor/common/core/offsetRange';
 import { IPosition, Position } from 'vs/editor/common/core/position';
 import { Range } from 'vs/editor/common/core/range';
 import { IChatAgentData, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
-import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart, IParsedChatRequest, IParsedChatRequestPart } from 'vs/workbench/contrib/chat/common/chatParserTypes';
+import { ChatRequestAgentPart, ChatRequestAgentSubcommandPart, ChatRequestSlashCommandPart, ChatRequestTextPart, ChatRequestVariablePart, IParsedChatRequest, IParsedChatRequestPart, chatVariableLeader } from 'vs/workbench/contrib/chat/common/chatParserTypes';
 import { IChatService } from 'vs/workbench/contrib/chat/common/chatService';
 import { IChatVariablesService } from 'vs/workbench/contrib/chat/common/chatVariables';
 
-const variableOrAgentReg = /^@([\w_\-]+)(:\d+)?(?=(\s|$|\b))/i; // An @-variable with an optional numeric : arg (@response:2)
+const agentReg = /^@([\w_\-]+)(?=(\s|$|\b))/i; // An @-agent
+const variableReg = /^#([\w_\-]+)(:\d+)?(?=(\s|$|\b))/i; // A #-variable with an optional numeric : arg (@response:2)
 const slashReg = /\/([\w_-]+)(?=(\s|$|\b))/i; // A / command
 
 export class ChatRequestParser {
@@ -31,8 +32,10 @@ export class ChatRequestParser {
 			const previousChar = message.charAt(i - 1);
 			const char = message.charAt(i);
 			let newPart: IParsedChatRequestPart | undefined;
-			if (char === '@' && (previousChar === ' ' || i === 0)) {
+			if (char === chatVariableLeader && (previousChar === ' ' || i === 0)) {
-				newPart = this.tryToParseVariableOrAgent(message.slice(i), i, new Position(lineNumber, column), parts);
+				newPart = this.tryToParseVariable(message.slice(i), i, new Position(lineNumber, column), parts);
+			} else if (char === '@' && (previousChar === ' ' || i === 0)) {
+				newPart = this.tryToParseAgent(message.slice(i), i, new Position(lineNumber, column), parts);
 			} else if (char === '/' && (previousChar === ' ' || i === 0)) {
 				// TODO try to make this sync
 				newPart = await this.tryToParseSlashCommand(sessionId, message.slice(i), i, new Position(lineNumber, column), parts);
@@ -64,10 +67,12 @@ export class ChatRequestParser {
 
 		const lastPart = parts.at(-1);
 		const lastPartEnd = lastPart?.range.endExclusive ?? 0;
+		if (lastPartEnd < message.length) {
 			parts.push(new ChatRequestTextPart(
 				new OffsetRange(lastPartEnd, message.length),
 				new Range(lastPart?.editorRange.endLineNumber ?? 1, lastPart?.editorRange.endColumn ?? 1, lineNumber, column),
 				message.slice(lastPartEnd, message.length)));
+		}
 
 		return {
 			parts,
@@ -75,8 +80,31 @@ export class ChatRequestParser {
 		};
 	}
 
-	private tryToParseVariableOrAgent(message: string, offset: number, position: IPosition, parts: ReadonlyArray<IParsedChatRequestPart>): ChatRequestAgentPart | ChatRequestVariablePart | undefined {
+	private tryToParseAgent(message: string, offset: number, position: IPosition, parts: ReadonlyArray<IParsedChatRequestPart>): ChatRequestAgentPart | ChatRequestVariablePart | undefined {
-		const nextVariableMatch = message.match(variableOrAgentReg);
+		const nextVariableMatch = message.match(agentReg);
+		if (!nextVariableMatch) {
+			return;
+		}
+
+		const [full, name] = nextVariableMatch;
+		const varRange = new OffsetRange(offset, offset + full.length);
+		const varEditorRange = new Range(position.lineNumber, position.column, position.lineNumber, position.column + full.length);
+
+		let agent: IChatAgentData | undefined;
+		if ((agent = this.agentService.getAgent(name))) {
+			if (parts.some(p => p instanceof ChatRequestAgentPart)) {
+				// Only one agent allowed
+				return;
+			} else {
+				return new ChatRequestAgentPart(varRange, varEditorRange, agent);
+			}
+		}
+
+		return;
+	}
+
+	private tryToParseVariable(message: string, offset: number, position: IPosition, parts: ReadonlyArray<IParsedChatRequestPart>): ChatRequestAgentPart | ChatRequestVariablePart | undefined {
+		const nextVariableMatch = message.match(variableReg);
 		if (!nextVariableMatch) {
 			return;
 		}
@@ -86,15 +114,7 @@ export class ChatRequestParser {
 		const varRange = new OffsetRange(offset, offset + full.length);
 		const varEditorRange = new Range(position.lineNumber, position.column, position.lineNumber, position.column + full.length);
 
-		let agent: IChatAgentData | undefined;
+		if (this.variableService.hasVariable(name)) {
-		if ((agent = this.agentService.getAgent(name)) && !variableArg) {
-			if (parts.some(p => p instanceof ChatRequestAgentPart)) {
-				// Only one agent allowed
-				return;
-			} else {
-				return new ChatRequestAgentPart(varRange, varEditorRange, agent);
-			}
-		} else if (this.variableService.hasVariable(name)) {
 			return new ChatRequestVariablePart(varRange, varEditorRange, name, variableArg);
 		}
 

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

@@ -9,7 +9,7 @@ import { Iterable } from 'vs/base/common/iterator';
 import { IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { IChatModel } from 'vs/workbench/contrib/chat/common/chatModel';
-import { ChatRequestVariablePart, IParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes';
+import { ChatRequestVariablePart, IParsedChatRequest, chatVariableLeader } from 'vs/workbench/contrib/chat/common/chatParserTypes';
 
 export interface IChatVariableData {
 	name: string;
@@ -74,7 +74,7 @@ export class ChatVariablesService implements IChatVariablesService {
 						jobs.push(data.resolver(prompt.text, varPart.variableArg, model, token).then(value => {
 							if (value) {
 								resolvedVariables[varPart.variableName] = value;
-								parsedPrompt[i] = `[@${varPart.variableName}](values:${varPart.variableName})`;
+								parsedPrompt[i] = `[${chatVariableLeader}${varPart.variableName}](values:${varPart.variableName})`;
 							} else {
 								parsedPrompt[i] = varPart.text;
 							}

src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_agents_and_variables_and_multiline.0.snap

@@ -48,16 +48,57 @@
     {
       range: {
         start: 29,
-        endExclusive: 63
+        endExclusive: 35
       },
       editorRange: {
         startLineNumber: 2,
         startColumn: 15,
+        endLineNumber: 2,
+        endColumn: 21
+      },
+      text: " with "
+    },
+    {
+      range: {
+        start: 35,
+        endExclusive: 45
+      },
+      editorRange: {
+        startLineNumber: 2,
+        startColumn: 21,
+        endLineNumber: 2,
+        endColumn: 31
+      },
+      variableName: "selection",
+      variableArg: ""
+    },
+    {
+      range: {
+        start: 45,
+        endExclusive: 50
+      },
+      editorRange: {
+        startLineNumber: 2,
+        startColumn: 31,
+        endLineNumber: 3,
+        endColumn: 5
+      },
+      text: "\nand "
+    },
+    {
+      range: {
+        start: 50,
+        endExclusive: 63
+      },
+      editorRange: {
+        startLineNumber: 3,
+        startColumn: 5,
         endLineNumber: 3,
         endColumn: 18
       },
-      text: " with @selection\nand @debugConsole"
+      variableName: "debugConsole",
+      variableArg: ""
     }
   ],
-  text: "@agent Please \ndo /subCommand with @selection\nand @debugConsole"
+  text: "@agent Please \ndo /subCommand with #selection\nand #debugConsole"
 }

src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_invalid_variables.0.snap

@@ -11,8 +11,8 @@
         endLineNumber: 1,
         endColumn: 27
       },
-      text: "What does @selection mean?"
+      text: "What does #selection mean?"
     }
   ],
-  text: "What does @selection mean?"
+  text: "What does #selection mean?"
 }

src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_variable_with_question_mark.0.snap

@@ -41,5 +41,5 @@
       text: "?"
     }
   ],
-  text: "What is @selection?"
+  text: "What is #selection?"
 }

src/vs/workbench/contrib/chat/test/common/__snapshots__/ChatRequestParser_variables.0.snap

@@ -41,5 +41,5 @@
       text: " mean?"
     }
   ],
-  text: "What does @selection mean?"
+  text: "What does #selection mean?"
 }

src/vs/workbench/contrib/chat/test/common/chatRequestParser.test.ts

@@ -82,7 +82,7 @@ suite('ChatRequestParser', () => {
 		instantiationService.stub(IChatVariablesService, variablesService as any);
 
 		parser = instantiationService.createInstance(ChatRequestParser);
-		const text = 'What does @selection mean?';
+		const text = 'What does #selection mean?';
 		const result = await parser.parseChatRequest('1', text);
 		await assertSnapshot(result);
 	});
@@ -93,7 +93,7 @@ suite('ChatRequestParser', () => {
 		instantiationService.stub(IChatVariablesService, variablesService as any);
 
 		parser = instantiationService.createInstance(ChatRequestParser);
-		const text = 'What is @selection?';
+		const text = 'What is #selection?';
 		const result = await parser.parseChatRequest('1', text);
 		await assertSnapshot(result);
 	});
@@ -104,7 +104,7 @@ suite('ChatRequestParser', () => {
 		instantiationService.stub(IChatVariablesService, variablesService as any);
 
 		parser = instantiationService.createInstance(ChatRequestParser);
-		const text = 'What does @selection mean?';
+		const text = 'What does #selection mean?';
 		const result = await parser.parseChatRequest('1', text);
 		await assertSnapshot(result);
 	});
@@ -149,7 +149,7 @@ suite('ChatRequestParser', () => {
 		instantiationService.stub(IChatVariablesService, variablesService as any);
 
 		parser = instantiationService.createInstance(ChatRequestParser);
-		const result = await parser.parseChatRequest('1', '@agent Please \ndo /subCommand with @selection\nand @debugConsole');
+		const result = await parser.parseChatRequest('1', '@agent Please \ndo /subCommand with #selection\nand #debugConsole');
 		await assertSnapshot(result);
 	});
 });

src/vs/workbench/contrib/chat/test/common/chatVariables.test.ts

@@ -44,43 +44,43 @@ suite('ChatVariables', function () {
 		};
 
 		{
-			const data = await resolveVariables('Hello @foo and@far');
+			const data = await resolveVariables('Hello #foo and#far');
 			assert.strictEqual(Object.keys(data.variables).length, 1);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.prompt, 'Hello [@foo](values:foo) and@far');
+			assert.strictEqual(data.prompt, 'Hello [#foo](values:foo) and#far');
 		}
 		{
-			const data = await resolveVariables('@foo Hello');
+			const data = await resolveVariables('#foo Hello');
 			assert.strictEqual(Object.keys(data.variables).length, 1);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.prompt, '[@foo](values:foo) Hello');
+			assert.strictEqual(data.prompt, '[#foo](values:foo) Hello');
 		}
 		{
-			const data = await resolveVariables('Hello @foo');
+			const data = await resolveVariables('Hello #foo');
 			assert.strictEqual(Object.keys(data.variables).length, 1);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
 		}
 		{
-			const data = await resolveVariables('Hello @foo?');
+			const data = await resolveVariables('Hello #foo?');
 			assert.strictEqual(Object.keys(data.variables).length, 1);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
-			assert.strictEqual(data.prompt, 'Hello [@foo](values:foo)?');
+			assert.strictEqual(data.prompt, 'Hello [#foo](values:foo)?');
 		}
 		{
-			const data = await resolveVariables('Hello @foo and@far @foo');
+			const data = await resolveVariables('Hello #foo and#far #foo');
 			assert.strictEqual(Object.keys(data.variables).length, 1);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['foo']);
 		}
 		{
-			const data = await resolveVariables('Hello @foo and @far @foo');
+			const data = await resolveVariables('Hello #foo and #far #foo');
 			assert.strictEqual(Object.keys(data.variables).length, 2);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['far', 'foo']);
 		}
 		{
-			const data = await resolveVariables('Hello @foo and @far @foo @unknown');
+			const data = await resolveVariables('Hello #foo and #far #foo #unknown');
 			assert.strictEqual(Object.keys(data.variables).length, 2);
 			assert.deepEqual(Object.keys(data.variables).sort(), ['far', 'foo']);
-			assert.strictEqual(data.prompt, 'Hello [@foo](values:foo) and [@far](values:far) [@foo](values:foo) @unknown');
+			assert.strictEqual(data.prompt, 'Hello [#foo](values:foo) and [#far](values:far) [#foo](values:foo) #unknown');
 		}
 
 		v1.dispose();