Change Create New Terminal to focus accordingly to the terminal location

[Abrifq]

Solves #237192 by using focusActiveTerminal.

I am not familliar with the codebase, so, please review if the else block is still needed.

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

Lines 313 to 325 in 9ded0d7

	run: async (c) => {
	if (c.service.isProcessSupportRegistered) {
	const instance = await c.service.createTerminal({ location: c.service.defaultLocation });
	if (!instance) {
	return;
	}
	c.service.setActiveInstance(instance);
	await focusActiveTerminal(instance, c);
	}
	else {
	await c.groupService.showPanel(true);
	}
	}

Have a nice day!

I'll undraft the PR as soon as it compiles on my machine and I verify it's working nicely.

[meganrogge]

Hi, thanks for working on this. From the code, I'm not sure it will work.

[Abrifq]

Hi! Thanks for replying.

It seems to work, feel free to test & review when you have time.

Also, should I update the branch when it's ready for review or when it's on merge queue? Doing so sounds like it will waste CI time.

[meganrogge]

shouldn't we be checking the location of the terminal to determine if we should focus the panel, vs doing so only when process support is not registered?

if (c.service.isProcessSupportRegistered) {

or perhaps the call to show panel is no longer needed?

[Abrifq]

shouldn't we be checking the location of the terminal to determine if we should focus the panel

focusActiveTerminal() does that already. I was going to write the same code myself but saw it's being used in commands like TerminalCommandId.New already and decided to use what's already here.

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

Lines 1708 to 1715 in e00c8eb

	async function focusActiveTerminal(instance: ITerminalInstance, c: ITerminalServicesCollection): Promise<void> {
	if (instance.target === TerminalLocation.Editor) {
	await c.editorService.revealActiveEditor();
	await instance.focusWhenReady(true);
	} else {
	await c.groupService.showPanel(true);
	}
	}

or perhaps the call to show panel is no longer needed?

That's why I wanted you to check, I'm not familliar with the codebase and I didn't research what's isProcessSupportRegistered implying.
Since I'm not familliar with what was on the mind when the function was writter and since focusActiveTerminal() already calls showPanel() when needed, I took it to an else clause.

(tl;dr: i don't know much better but it should be logically same while not annoying me when it spawns a terminal on editor area)

[meganrogge]

Thanks!