onDidExecuteTerminalCommand does not fire events for hidden terminal

[karrtikr]

Does this issue occur when all extensions are disabled?: Yes/No

For proposed API: #145234

Version: 1.85.0-insider (user setup)
OS: Windows_NT x64 10.0.22518

Steps to Reproduce:

1. Create a hidden terminal using hideFromUser to true
2. Send text to terminal, for eg. "echo hello"
3. Watch no events are fired

Same is not the case if hideFromUser is set to false.

cc/ @Tyriar

[karrtikr]

@Tyriar @meganrogge Any idea where the issue could be here? We require this to start leveraging #145234.

[Tyriar]

This is by design according to this:

vscode/src/vs/platform/terminal/node/terminalEnvironment.ts

Lines 120 to 122 in 08159ae

	if (!options.shellIntegration.enabled || !shellLaunchConfig.executable || (shellLaunchConfig.isFeatureTerminal && !shellLaunchConfig.forceShellIntegration) || shellLaunchConfig.hideFromUser || shellLaunchConfig.ignoreShellIntegration || useWinpty) {
	return undefined;
	}

We tweaked the condition in #206543 to work properly on remotes, and tried to force shell integration for debug terminals here:

vscode/src/vs/workbench/api/node/extHostDebugService.ts

Lines 111 to 113 in 08159ae

	// Since debug termnials are REPLs, we want shell integration to be enabled.
	// Ignore isFeatureTerminal when evaluating shell integration enablement.
	forceShellIntegration: true,

I don't see an issue with changing hidden terminals from having shell integration, we just need to watch the incoming issues for problems.

[Tyriar]

If we have shell integration in hidden terminals is conflicts with #197187 (comment) but I think the thinking was changed in #194819 (comment)?

If you don't want env collections you must pass strictEnv, which means you need to provide the entire environment.

[TylerLeonhardt]

I still repro this with the following minimum repro:

import * as vscode from 'vscode';

export async function activate(context: vscode.ExtensionContext) {
	const terminal = vscode.window.createTerminal({
		hideFromUser: true,
		shellPath: '/bin/bash',
	});
	vscode.window.onDidExecuteTerminalCommand(e=> {
		// Does not fire
		vscode.window.showInformationMessage(e.commandLine ?? '');
	});
	terminal.sendText('echo "hello"');
}

Changing to hideFromUser: false fires the event.

Or maybe there are other steps that should be taken?

[meganrogge]

Reopening as your steps seem correct @TylerLeonhardt

[meganrogge]

possible that the hidden terminal isn't yet a part of terminalGroupInstances yet, so is not within this.instances here?

vscode/src/vs/workbench/contrib/terminal/browser/terminalService.ts

Line 95 in 864554b

return this._terminalGroupService.instances.concat(this._terminalEditorService.instances);

vscode/src/vs/workbench/contrib/terminal/browser/terminalService.ts

Line 1193 in 864554b

return createInstanceCapabilityEventMultiplexer(this.instances, this.onDidCreateInstance, this.onDidDisposeInstance, capabilityId, getEvent);

Yes, seems like it

vscode/src/vs/workbench/contrib/terminal/browser/terminalService.ts

Lines 368 to 373 in 864554b

	setActiveInstance(value: ITerminalInstance) {
	// If this was a hideFromUser terminal created by the API this was triggered by show,
	// in which case we need to create the terminal group
	if (value.shellLaunchConfig.hideFromUser) {
	this._showBackgroundTerminal(value);
	}

[meganrogge]

We should include _backgroundedTerminalInstances in instances I think

[vscodenpa]

Friendly ping! Looks like this issue requires some further steps to be verified. Please provide us with the steps necessary to verify this issue.

[meganrogge]

@eleanorjboyd these steps are good

#199611 (comment)