Skip to content

Commit fc298c2

Browse files
committed
Re-write && to ; in pwsh
Fixes microsoft#272704
1 parent f2529aa commit fc298c2

File tree

4 files changed

+59
-8
lines changed

4 files changed

+59
-8
lines changed

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandSimplifier.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,32 @@ import { IWorkspaceContextService } from '../../../../../platform/workspace/comm
99
import type { ITerminalInstance } from '../../../terminal/browser/terminal.js';
1010
import { isPowerShell } from './runInTerminalHelpers.js';
1111
import type { IRunInTerminalInputParams } from './tools/runInTerminalTool.js';
12+
import { TreeSitterCommandParserLanguage, type TreeSitterCommandParser } from './treeSitterCommandParser.js';
1213

1314
export class CommandSimplifier {
1415
constructor(
1516
private readonly _osBackend: Promise<OperatingSystem>,
17+
private readonly _treeSitterCommandParser: TreeSitterCommandParser,
1618
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
1719
) {
1820
}
1921

2022
async rewriteIfNeeded(args: IRunInTerminalInputParams, instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, shell: string): Promise<string> {
21-
const commandLine = args.command;
2223
const os = await this._osBackend;
2324

25+
let commandLine = args.command;
26+
commandLine = await this._removeRedundantCdPrefix(instance, commandLine, os, shell);
27+
commandLine = await this._rewriteUnsupportedPwshChainOperators(commandLine, os, shell);
28+
29+
return commandLine;
30+
}
31+
32+
private async _removeRedundantCdPrefix(instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, commandLine: string, os: OperatingSystem, shell: string): Promise<string> {
33+
const isPwsh = isPowerShell(shell, os);
34+
2435
// Re-write the command if it starts with `cd <dir> && <suffix>` or `cd <dir>; <suffix>`
2536
// to just `<suffix>` if the directory matches the current terminal's cwd. This simplifies
2637
// the result in the chat by removing redundancies that some models like to add.
27-
const isPwsh = isPowerShell(shell, os);
2838
const cdPrefixMatch = commandLine.match(
2939
isPwsh
3040
? /^(?:cd(?: \/d)?|Set-Location(?: -Path)?) (?<dir>[^\s]+) ?(?:&&|;)\s+(?<suffix>.+)$/i
@@ -68,7 +78,25 @@ export class CommandSimplifier {
6878
}
6979
}
7080
}
81+
return commandLine;
82+
}
7183

84+
private async _rewriteUnsupportedPwshChainOperators(commandLine: string, os: OperatingSystem, shell: string) {
85+
// TODO: This should just be Windows PowerShell in the future when the powershell grammar
86+
// supports chain operators https://github.com/airbus-cert/tree-sitter-powershell/issues/27
87+
if (isPowerShell(shell, os)) {
88+
const doubleAmpersandCaptures = await this._treeSitterCommandParser.queryTree(TreeSitterCommandParserLanguage.PowerShell, commandLine, [
89+
'(',
90+
' (command',
91+
' (command_elements',
92+
' (generic_token) @double.ampersand',
93+
' (#eq? @double.ampersand "&&")))',
94+
')',
95+
].join('\n'));
96+
for (const capture of doubleAmpersandCaptures.reverse()) {
97+
commandLine = `${commandLine.substring(0, capture.node.startIndex)};${commandLine.substring(capture.node.endIndex)}`;
98+
}
99+
}
72100
return commandLine;
73101
}
74102
}

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ function createPowerShellModelDescription(shell: string): string {
5656
`This tool allows you to execute ${isWinPwsh ? 'Windows PowerShell 5.1' : 'PowerShell'} commands in a persistent terminal session, preserving environment variables, working directory, and other context across multiple commands.`,
5757
'',
5858
'Command Execution:',
59-
// Even for pwsh 7+ we want to use `;` to chain commands since the tree sitter grammar
60-
// doesn't parse `&&`. See https://github.com/airbus-cert/tree-sitter-powershell/issues/27
59+
// TODO: Even for pwsh 7+ we want to use `;` to chain commands since the tree sitter grammar
60+
// doesn't parse `&&`. We want to change this to avoid `&&` only in Windows PowerShell when
61+
// the grammar supports it https://github.com/airbus-cert/tree-sitter-powershell/issues/27
6162
'- Use semicolons ; to chain commands on one line, NEVER use && even when asked explicitly',
6263
'- Prefer pipelines | for object-based data flow',
6364
'',
@@ -292,8 +293,8 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
292293
this._osBackend = this._remoteAgentService.getEnvironment().then(remoteEnv => remoteEnv?.os ?? OS);
293294

294295
this._terminalToolCreator = _instantiationService.createInstance(ToolTerminalCreator);
295-
this._commandSimplifier = _instantiationService.createInstance(CommandSimplifier, this._osBackend);
296296
this._treeSitterCommandParser = this._instantiationService.createInstance(TreeSitterCommandParser);
297+
this._commandSimplifier = _instantiationService.createInstance(CommandSimplifier, this._osBackend, this._treeSitterCommandParser);
297298
this._telemetry = _instantiationService.createInstance(RunInTerminalToolTelemetry);
298299
this._commandLineAutoApprover = this._register(_instantiationService.createInstance(CommandLineAutoApprover));
299300
this._profileFetcher = _instantiationService.createInstance(TerminalProfileFetcher);

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/treeSitterCommandParser.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { BugIndicatingError } from '../../../../../base/common/errors.js';
77
import { derived, waitForState } from '../../../../../base/common/observable.js';
88
import { ITreeSitterLibraryService } from '../../../../../editor/common/services/treeSitter/treeSitterLibraryService.js';
9-
import type { Language, Parser, Query } from '@vscode/tree-sitter-wasm';
9+
import type { Language, Parser, Query, QueryCapture } from '@vscode/tree-sitter-wasm';
1010

1111
export const enum TreeSitterCommandParserLanguage {
1212
Bash = 'bash',
@@ -42,9 +42,32 @@ export class TreeSitterCommandParser {
4242

4343
const captures = query.captures(tree.rootNode);
4444
const subCommands = captures.map(e => e.node.text);
45+
4546
return subCommands;
4647
}
4748

49+
async queryTree(languageId: TreeSitterCommandParserLanguage, commandLine: string, querySource: string): Promise<QueryCapture[]> {
50+
const parser = await this._parser;
51+
const language = await waitForState(derived(reader => {
52+
return this._treeSitterLibraryService.getLanguage(languageId, true, reader);
53+
}));
54+
parser.setLanguage(language);
55+
56+
const tree = parser.parse(commandLine);
57+
if (!tree) {
58+
throw new BugIndicatingError('Failed to parse tree');
59+
}
60+
61+
const query = await this._treeSitterLibraryService.createQuery(language, querySource);
62+
if (!query) {
63+
throw new BugIndicatingError('Failed to create tree sitter query');
64+
}
65+
66+
const captures = query.captures(tree.rootNode);
67+
68+
return captures;
69+
}
70+
4871
private async _getQuery(language: Language): Promise<Query> {
4972
let query = this._queries.get(language);
5073
if (!query) {

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/treeSitterCommandParser.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ suite('TreeSitterCommandParser', () => {
2222
let parser: TreeSitterCommandParser;
2323

2424
setup(() => {
25-
const logService = new NullLogService();
26-
const fileService = store.add(new FileService(logService));
25+
const fileService = store.add(new FileService(new NullLogService()));
2726
const fileSystemProvider = new TestIPCFileSystemProvider();
2827
store.add(fileService.registerProvider(Schemas.file, fileSystemProvider));
2928

0 commit comments

Comments
 (0)