Skip to content

Fix same-command buttons sharing terminal #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kubrickcode merged 1 commit into from
Oct 30, 2025
Merged

Fix same-command buttons sharing terminal #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/extension/src/adapters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ const DEFAULT_REFRESH_CONFIG: RefreshButtonConfig = {
export type TerminalExecutor = (
command: string,
useVsCodeApi?: boolean,
terminalName?: string
terminalName?: string,
buttonName?: string,
buttonRef?: object
) => void;

export type ConfigReader = {
Expand Down
37 changes: 19 additions & 18 deletions src/extension/src/command-executor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ describe("command-executor", () => {

executeTerminalCommand(button, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", false, undefined);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", false, undefined, "Test Button", expect.objectContaining({ command: "echo test", name: "Test Button" }));
});

it("should call terminalExecutor with useVsCodeApi true", () => {
Expand All @@ -511,7 +511,7 @@ describe("command-executor", () => {

executeTerminalCommand(button, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", true, undefined);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", true, undefined, "Test Button", expect.objectContaining({ command: "echo test", name: "Test Button", useVsCodeApi: true }));
});

it("should call terminalExecutor with custom terminal name", () => {
Expand All @@ -524,7 +524,7 @@ describe("command-executor", () => {

executeTerminalCommand(button, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", false, "Custom Terminal");
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", false, "Custom Terminal", "Test Button", expect.objectContaining({ command: "echo test", name: "Test Button", terminalName: "Custom Terminal" }));
});

it("should call terminalExecutor with all parameters", () => {
Expand All @@ -538,7 +538,7 @@ describe("command-executor", () => {

executeTerminalCommand(button, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", true, "Custom Terminal");
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo test", true, "Custom Terminal", "Test Button", expect.objectContaining({ command: "echo test", name: "Test Button", terminalName: "Custom Terminal", useVsCodeApi: true }));
});

it("should not call terminalExecutor when command is undefined", () => {
Expand Down Expand Up @@ -588,13 +588,14 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(3);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo test1", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo test2", true, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo test1", false, undefined, "Command 1[0]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo test2", true, undefined, "Command 2[1]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(
3,
"echo test3",
false,
"Custom Terminal"
"Custom Terminal",
"Command 3[2]"
);
});

Expand All @@ -621,8 +622,8 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(2);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo child1", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo child2", true, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo child1", false, undefined, "Group Command[0]>Child 1[0]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo child2", true, undefined, "Group Command[0]>Child 2[1]");
});

it("should not execute commands for buttons with groups but no executeAll flag", () => {
Expand Down Expand Up @@ -673,8 +674,8 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(2);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo level3", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo level2", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo level3", false, undefined, "Level 1 Group[0]>Level 2 Group[0]>Level 3 Command[0]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo level2", false, undefined, "Level 1 Group[0]>Level 2 Command[1]");
});

it("should skip buttons without commands and without groups", () => {
Expand All @@ -692,7 +693,7 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(1);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo valid", false, undefined);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo valid", false, undefined, "Valid Command[0]");
});

it("should skip buttons with empty command strings", () => {
Expand All @@ -711,7 +712,7 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(1);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo valid", false, undefined);
expect(mockTerminalExecutor).toHaveBeenCalledWith("echo valid", false, undefined, "Valid Command[0]");
});

it("should handle empty commands array", () => {
Expand Down Expand Up @@ -758,8 +759,8 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(2);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo regular", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo child", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo regular", false, undefined, "Regular Command[0]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo child", false, undefined, "Group with executeAll[1]>Child Command[0]");
});

it("should handle complex nested structure with mixed executeAll flags", () => {
Expand Down Expand Up @@ -804,9 +805,9 @@ describe("command-executor", () => {
executeCommandsRecursively(commands, mockTerminalExecutor);

expect(mockTerminalExecutor).toHaveBeenCalledTimes(3);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo leaf1", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo leaf2", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(3, "echo direct", false, undefined);
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(1, "echo leaf1", false, undefined, "Root Group[0]>Branch 1[0]>Leaf 1[0]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(2, "echo leaf2", false, undefined, "Root Group[0]>Branch 1[0]>Leaf 2[1]");
expect(mockTerminalExecutor).toHaveBeenNthCalledWith(3, "echo direct", false, undefined, "Root Group[0]>Direct Command[2]");
});
});
});
21 changes: 15 additions & 6 deletions src/extension/src/command-executor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ export const executeTerminalCommand = (
) => {
if (!button.command) return;

terminalExecutor(button.command, button.useVsCodeApi || false, button.terminalName);
terminalExecutor(
button.command,
button.useVsCodeApi || false,
button.terminalName,
button.name,
button
);
};

export const executeButtonCommand = (
Expand Down Expand Up @@ -187,19 +193,22 @@ const showGroupQuickPick = (

export const executeCommandsRecursively = (
commands: ButtonConfig[],
terminalExecutor: TerminalExecutor
terminalExecutor: TerminalExecutor,
parentPath = ""
): void => {
Comment thread
kubrickcode marked this conversation as resolved.
commands.forEach((cmd) => {
commands.forEach((cmd, index) => {
const buttonId = parentPath ? `${parentPath}>${cmd.name}[${index}]` : `${cmd.name}[${index}]`;

if (cmd.group && cmd.executeAll) {
executeCommandsRecursively(cmd.group, terminalExecutor);
executeCommandsRecursively(cmd.group, terminalExecutor, buttonId);
} else if (cmd.command) {
terminalExecutor(cmd.command, cmd.useVsCodeApi || false, cmd.terminalName);
terminalExecutor(cmd.command, cmd.useVsCodeApi || false, cmd.terminalName, buttonId);
}
});
};

const executeAllCommands = (button: ButtonConfig, terminalExecutor: TerminalExecutor) => {
if (!button.group) return;

executeCommandsRecursively(button.group, terminalExecutor);
executeCommandsRecursively(button.group, terminalExecutor, button.name);
};
27 changes: 18 additions & 9 deletions src/extension/src/command-tree-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ConfigReader, TerminalExecutor } from "./adapters";
import { ButtonConfig } from "./types";

export class CommandTreeItem extends vscode.TreeItem {
public readonly buttonName: string;
public readonly commandString: string;
public readonly terminalName?: string;
public readonly useVsCodeApi: boolean;
Expand All @@ -11,12 +12,14 @@ export class CommandTreeItem extends vscode.TreeItem {
label: string,
commandString: string,
useVsCodeApi: boolean = false,
terminalName?: string
terminalName?: string,
buttonName?: string
) {
super(label, vscode.TreeItemCollapsibleState.None);
this.commandString = commandString;
this.useVsCodeApi = useVsCodeApi;
this.terminalName = terminalName;
this.buttonName = buttonName || label;
this.tooltip = commandString;
this.contextValue = "command";
this.command = {
Expand All @@ -38,8 +41,10 @@ export class GroupTreeItem extends vscode.TreeItem {

type TreeItem = CommandTreeItem | GroupTreeItem;

export const createTreeItemsFromGroup = (commands: ButtonConfig[]): TreeItem[] => {
return commands.map((cmd) => {
export const createTreeItemsFromGroup = (commands: ButtonConfig[], parentPath = ""): TreeItem[] => {
return commands.map((cmd, index) => {
Comment thread
kubrickcode marked this conversation as resolved.
const buttonId = parentPath ? `${parentPath}>${cmd.name}[${index}]` : `${cmd.name}[${index}]`;

if (cmd.group) {
return new GroupTreeItem(cmd.name, cmd.group);
}
Expand All @@ -48,13 +53,16 @@ export const createTreeItemsFromGroup = (commands: ButtonConfig[]): TreeItem[] =
cmd.name,
cmd.command || "",
cmd.useVsCodeApi || false,
cmd.terminalName
cmd.terminalName,
buttonId
);
});
};

export const createRootTreeItems = (buttons: ButtonConfig[]): TreeItem[] => {
return buttons.map((button) => {
return buttons.map((button, index) => {
const buttonId = `${button.name}[${index}]`;

if (button.group) {
return new GroupTreeItem(button.name, button.group);
}
Expand All @@ -64,11 +72,12 @@ export const createRootTreeItems = (buttons: ButtonConfig[]): TreeItem[] => {
button.name,
button.command,
button.useVsCodeApi || false,
button.terminalName
button.terminalName,
buttonId
);
}

return new CommandTreeItem(button.name, "", false);
return new CommandTreeItem(button.name, "", false, undefined, buttonId);
});
};

Expand All @@ -82,7 +91,7 @@ export class CommandTreeProvider implements vscode.TreeDataProvider<TreeItem> {
new CommandTreeProvider(configReader);

static executeFromTree = (item: CommandTreeItem, terminalExecutor: TerminalExecutor) => {
terminalExecutor(item.commandString, item.useVsCodeApi, item.terminalName);
terminalExecutor(item.commandString, item.useVsCodeApi, item.terminalName, item.buttonName);
};

getChildren = (element?: TreeItem): Thenable<TreeItem[]> => {
Expand All @@ -91,7 +100,7 @@ export class CommandTreeProvider implements vscode.TreeDataProvider<TreeItem> {
}

if (element instanceof GroupTreeItem) {
return Promise.resolve(createTreeItemsFromGroup(element.commands));
return Promise.resolve(createTreeItemsFromGroup(element.commands, element.label));
}

return Promise.resolve([]);
Expand Down
39 changes: 23 additions & 16 deletions src/extension/src/terminal-manager.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,36 +78,43 @@ describe("terminal-manager", () => {
jest.restoreAllMocks();
});

it("should create separate terminals when customTerminalName is set", () => {
manager.executeCommand("npm start", false, "build");
manager.executeCommand("npm test", false, "build");
it("should create separate terminals for different buttonNames with same command", () => {
manager.executeCommand("npm start", false, "build", "Button A");
manager.executeCommand("npm start", false, "build", "Button B");

expect(vscode.window.createTerminal).toHaveBeenCalledTimes(2);
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(1, "build");
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(2, "build");
});

it("should create new terminal every time when customTerminalName is set", () => {
manager.executeCommand("npm start", false, "build");
manager.executeCommand("npm start", false, "build");
it("should reuse terminal for same button configuration", () => {
manager.executeCommand("npm start", false, "build", "Button A");
manager.executeCommand("npm start", false, "build", "Button A");

expect(vscode.window.createTerminal).toHaveBeenCalledTimes(2);
expect(vscode.window.createTerminal).toHaveBeenCalledTimes(1);
});

it("should reuse terminal for same command without customTerminalName", () => {
manager.executeCommand("npm start", false);
manager.executeCommand("npm start", false);
it("should create separate terminals for same command with different terminalNames", () => {
manager.executeCommand("just test", false, "", "just test");
manager.executeCommand("just test", false, undefined, "just test");

expect(vscode.window.createTerminal).toHaveBeenCalledTimes(1);
expect(vscode.window.createTerminal).toHaveBeenCalledTimes(2);
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(1, "just");
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(2, "just");
});

it("should create separate terminals for different commands without customTerminalName", () => {
manager.executeCommand("npm start", false);
manager.executeCommand("npm test", false);
it("should create separate terminals for executeAll group with same command", () => {
manager.executeCommand("just test", false, "", "just test 1");
manager.executeCommand("just test", false, undefined, "just test 2");

expect(vscode.window.createTerminal).toHaveBeenCalledTimes(2);
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(1, "npm");
expect(vscode.window.createTerminal).toHaveBeenNthCalledWith(2, "npm");
});

it("should reuse terminal when same button is clicked again", () => {
manager.executeCommand("npm test", false, undefined, "Test Button");
manager.executeCommand("npm test", false, undefined, "Test Button");

expect(vscode.window.createTerminal).toHaveBeenCalledTimes(1);
});
Comment thread
kubrickcode marked this conversation as resolved.
});
});
41 changes: 31 additions & 10 deletions src/extension/src/terminal-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export const determineTerminalName = (
};

export class TerminalManager {
private buttonIds = new WeakMap<object, string>();
private idCounter = 0;
private terminals = new Map<string, vscode.Terminal>();

static create = (): TerminalManager => new TerminalManager();
Expand All @@ -24,29 +26,48 @@ export class TerminalManager {
this.terminals.clear();
};

executeCommand: TerminalExecutor = (command, useVsCodeApi = false, customTerminalName) => {
executeCommand: TerminalExecutor = (
command,
useVsCodeApi = false,
customTerminalName,
buttonName,
buttonRef
) => {
Comment thread
kubrickcode marked this conversation as resolved.
if (useVsCodeApi) {
vscode.commands.executeCommand(command);
return;
}

const terminalName = determineTerminalName(customTerminalName, command);
const uniqueId = this.getUniqueButtonId(buttonRef, buttonName);
const terminalKey = JSON.stringify({
command,
name: uniqueId,
terminalName: customTerminalName,
useVsCodeApi,
});

if (customTerminalName) {
const terminal = vscode.window.createTerminal(terminalName);
terminal.show();
terminal.sendText(command);
return;
}

let terminal = this.terminals.get(command);
let terminal = this.terminals.get(terminalKey);

if (shouldCreateNewTerminal(terminal)) {
terminal = vscode.window.createTerminal(terminalName);
this.terminals.set(command, terminal);
this.terminals.set(terminalKey, terminal);
}

terminal!.show();
terminal!.sendText(command);
};

private getUniqueButtonId(buttonRef?: object, buttonName?: string): string {
if (buttonRef) {
let id = this.buttonIds.get(buttonRef);
if (!id) {
id = `btn-${this.idCounter++}`;
this.buttonIds.set(buttonRef, id);
}
return id;
}

return buttonName ?? `temp-${this.idCounter++}`;
}
}