Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function TabItem({ tab, childTabs = [] }: TabItemProps) {
onClick={handleTabClick}
onDoubleClick={rename.startRename}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
if (!rename.isRenaming && (e.key === "Enter" || e.key === " ")) {
e.preventDefault();
handleTabClick();
}
Expand Down
6 changes: 3 additions & 3 deletions apps/desktop/src/renderer/stores/tabs/drag-logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export const handleDragTabToTab = (

// Rule 1: Dragging tab into itself - creates new tab and makes a group
if (draggedTabId === targetTabId) {
const newTab = createNewTab(workspaceId, TabType.Single);
const newTab = createNewTab(workspaceId, TabType.Single, state.tabs);

// If dragged tab is a child tab, add new tab to its parent group
if (draggedTab.parentId) {
Expand Down Expand Up @@ -264,7 +264,7 @@ export const handleDragTabToTab = (

// If dragging a child tab into another child tab of the same group, create a new tab
if (draggedTab.parentId === parentGroup.id) {
const newTab = createNewTab(workspaceId, TabType.Single);
const newTab = createNewTab(workspaceId, TabType.Single, state.tabs);

const updatedNewTab: Tab = {
...newTab,
Expand Down Expand Up @@ -327,7 +327,7 @@ export const handleDragTabToTab = (
if (targetTab.type === TabType.Group && draggedTab.type === TabType.Single) {
// If dragging a tab from the same group, create a new tab and add to the group
if (draggedTab.parentId === targetTabId) {
const newTab = createNewTab(workspaceId, TabType.Single);
const newTab = createNewTab(workspaceId, TabType.Single, state.tabs);

const updatedNewTab: Tab = {
...newTab,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const splitPaneInGroup = (
);
if (!group || group.type !== TabType.Group || !group.layout) return state;

const newTab = createNewTab(workspaceId, TabType.Single);
const newTab = createNewTab(workspaceId, TabType.Single, state.tabs);
const newTabWithParent: Tab = {
...newTab,
parentId: tabToSplit.parentId,
Expand Down Expand Up @@ -106,13 +106,10 @@ const convertTabToGroup = (
isNew: false,
};

const newChildTab: Tab = {
id: `tab-${Date.now() + 1}-${Math.random().toString(36).substring(2, 11)}`,
title: "New Tab",
workspaceId,
type: TabType.Single,
const newChildTab = createNewTab(workspaceId, TabType.Single, state.tabs);
const newChildTabWithParent: Tab = {
...newChildTab,
parentId: groupTab.id,
isNew: true,
};

const updatedSourceTab: Tab = {
Expand All @@ -123,7 +120,7 @@ const convertTabToGroup = (
const layout: MosaicNode<string> = {
direction,
first: tabToSplit.id,
second: newChildTab.id,
second: newChildTabWithParent.id,
splitPercentage: 50,
};

Expand Down Expand Up @@ -152,7 +149,7 @@ const convertTabToGroup = (
...nonWorkspaceTabs,
...otherWorkspaceTabs,
updatedSourceTab,
newChildTab,
newChildTabWithParent,
];

return {
Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/renderer/stores/tabs/helpers/tab-crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const handleAddTab = (
workspaceId: string,
type: TabType = TabType.Single,
): Partial<TabsState> => {
const newTab = createNewTab(workspaceId, type);
const newTab = createNewTab(workspaceId, type, state.tabs);
const currentActiveId = state.activeTabIds[workspaceId];
const historyStack = state.tabHistoryStacks[workspaceId] || [];
const newHistoryStack = currentActiveId
Expand Down
12 changes: 11 additions & 1 deletion apps/desktop/src/renderer/stores/tabs/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type Tab, TabType } from "./types";
import { generateTerminalName } from "./utils/terminal-naming";

/**
* Helper function to get child tab IDs for a given parent ID
Expand All @@ -10,11 +11,20 @@ export const getChildTabIds = (tabs: Tab[], parentId: string): string[] => {
export const createNewTab = (
workspaceId: string,
type: TabType = TabType.Single,
existingTabs: Tab[] = [],
): Tab => {
const id = `tab-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`;

// Generate unique terminal name based on existing tabs
const existingNames = existingTabs.map((tab) => tab.title);
const title =
type === TabType.Single
? generateTerminalName(existingNames)
: "New Split View";

const baseTab = {
id,
title: type === TabType.Single ? "New Terminal" : "New Split View",
title,
workspaceId,
isNew: true,
};
Expand Down
152 changes: 152 additions & 0 deletions apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
import { describe, expect, it } from "bun:test";
import {
DefaultTerminalNamingStrategy,
generateTerminalName,
type TerminalNamingStrategy,
} from "./terminal-naming";

describe("DefaultTerminalNamingStrategy", () => {
it("should return base name when no collision", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName([]);
expect(result).toBe("Terminal");
});

it("should return base name when no collision with other names", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName(["Other Name", "Another Terminal"]);
expect(result).toBe("Terminal");
});

it("should return Terminal (1) when base name exists", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName(["Terminal"]);
expect(result).toBe("Terminal (1)");
});

it("should increment number on multiple collisions", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName([
"Terminal",
"Terminal (1)",
"Terminal (2)",
]);
expect(result).toBe("Terminal (3)");
});

it("should find gaps in numbering", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName(["Terminal", "Terminal (2)"]);
expect(result).toBe("Terminal (1)");
});

it("should handle non-sequential numbers", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName([
"Terminal",
"Terminal (5)",
"Terminal (10)",
]);
expect(result).toBe("Terminal (1)");
});

it("should handle custom base name", () => {
const strategy = new DefaultTerminalNamingStrategy("Shell");
const result = strategy.generateName([]);
expect(result).toBe("Shell");
});

it("should handle custom base name with collision", () => {
const strategy = new DefaultTerminalNamingStrategy("Shell");
const result = strategy.generateName(["Shell", "Shell (1)"]);
expect(result).toBe("Shell (2)");
});

it("should handle large numbers of terminals", () => {
const strategy = new DefaultTerminalNamingStrategy();
const existingNames = [
"Terminal",
...Array.from({ length: 100 }, (_, i) => `Terminal (${i + 1})`),
];
const result = strategy.generateName(existingNames);
expect(result).toBe("Terminal (101)");
});

it("should not be affected by similar named terminals", () => {
const strategy = new DefaultTerminalNamingStrategy();
const result = strategy.generateName([
"Terminal Window",
"My Terminal",
"Terminal (x)",
]);
expect(result).toBe("Terminal");
});
});

describe("generateTerminalName helper", () => {
it("should use default strategy when no strategy provided", () => {
const result = generateTerminalName([]);
expect(result).toBe("Terminal");
});

it("should use default strategy with collisions", () => {
const result = generateTerminalName(["Terminal", "Terminal (1)"]);
expect(result).toBe("Terminal (2)");
});

it("should accept custom strategy", () => {
class CustomStrategy implements TerminalNamingStrategy {
generateName(_existingNames: string[]): string {
return "Custom Terminal";
}
}

const result = generateTerminalName(["Terminal"], new CustomStrategy());
expect(result).toBe("Custom Terminal");
});
});

describe("Terminal naming in realistic scenarios", () => {
it("should generate sequential names for multiple new terminals", () => {
const strategy = new DefaultTerminalNamingStrategy();
const existingNames: string[] = [];

const name1 = strategy.generateName(existingNames);
expect(name1).toBe("Terminal");
existingNames.push(name1);

const name2 = strategy.generateName(existingNames);
expect(name2).toBe("Terminal (1)");
existingNames.push(name2);

const name3 = strategy.generateName(existingNames);
expect(name3).toBe("Terminal (2)");
existingNames.push(name3);

expect(existingNames).toEqual(["Terminal", "Terminal (1)", "Terminal (2)"]);
});

it("should reuse numbers after terminal is closed", () => {
const strategy = new DefaultTerminalNamingStrategy();
let existingNames = ["Terminal", "Terminal (1)", "Terminal (2)"];

// Close Terminal (1)
existingNames = existingNames.filter((name) => name !== "Terminal (1)");

const newName = strategy.generateName(existingNames);
expect(newName).toBe("Terminal (1)");
});

it("should handle mixed terminal types", () => {
const strategy = new DefaultTerminalNamingStrategy();
const existingNames = [
"Terminal",
"New Split View",
"Terminal (1)",
"Another Tab",
];

const result = strategy.generateName(existingNames);
expect(result).toBe("Terminal (2)");
});
});
46 changes: 46 additions & 0 deletions apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* Terminal naming utility
* Generates unique terminal names with incremental numbering only on collision
*/

export interface TerminalNamingStrategy {
generateName(existingNames: string[]): string;
}

export class DefaultTerminalNamingStrategy implements TerminalNamingStrategy {
private readonly baseName: string;

constructor(baseName = "Terminal") {
this.baseName = baseName;
}

/**
* Generate a unique terminal name
* - Returns "Terminal" if no collision
* - Returns "Terminal (1)", "Terminal (2)", etc. on collision
*/
generateName(existingNames: string[]): string {
// If no collision with base name, use it
if (!existingNames.includes(this.baseName)) {
return this.baseName;
}

// Find the next available number
let counter = 1;
while (existingNames.includes(`${this.baseName} (${counter})`)) {
counter++;
}

return `${this.baseName} (${counter})`;
}
}

/**
* Helper function to generate a unique terminal name
*/
export const generateTerminalName = (
existingNames: string[],
strategy: TerminalNamingStrategy = new DefaultTerminalNamingStrategy(),
): string => {
return strategy.generateName(existingNames);
};
Loading