diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx index 330ee95c789..727d5396fb3 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/TabsView/TabItem/index.tsx @@ -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(); } diff --git a/apps/desktop/src/renderer/stores/tabs/drag-logic.ts b/apps/desktop/src/renderer/stores/tabs/drag-logic.ts index f1fa2739525..3f1bd5a3e39 100644 --- a/apps/desktop/src/renderer/stores/tabs/drag-logic.ts +++ b/apps/desktop/src/renderer/stores/tabs/drag-logic.ts @@ -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) { @@ -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, @@ -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, diff --git a/apps/desktop/src/renderer/stores/tabs/helpers/split-operations.ts b/apps/desktop/src/renderer/stores/tabs/helpers/split-operations.ts index da96449197b..00930e5ade6 100644 --- a/apps/desktop/src/renderer/stores/tabs/helpers/split-operations.ts +++ b/apps/desktop/src/renderer/stores/tabs/helpers/split-operations.ts @@ -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, @@ -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 = { @@ -123,7 +120,7 @@ const convertTabToGroup = ( const layout: MosaicNode = { direction, first: tabToSplit.id, - second: newChildTab.id, + second: newChildTabWithParent.id, splitPercentage: 50, }; @@ -152,7 +149,7 @@ const convertTabToGroup = ( ...nonWorkspaceTabs, ...otherWorkspaceTabs, updatedSourceTab, - newChildTab, + newChildTabWithParent, ]; return { diff --git a/apps/desktop/src/renderer/stores/tabs/helpers/tab-crud.ts b/apps/desktop/src/renderer/stores/tabs/helpers/tab-crud.ts index 67a2264a7db..5355c8a6889 100644 --- a/apps/desktop/src/renderer/stores/tabs/helpers/tab-crud.ts +++ b/apps/desktop/src/renderer/stores/tabs/helpers/tab-crud.ts @@ -8,7 +8,7 @@ export const handleAddTab = ( workspaceId: string, type: TabType = TabType.Single, ): Partial => { - 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 diff --git a/apps/desktop/src/renderer/stores/tabs/utils.ts b/apps/desktop/src/renderer/stores/tabs/utils.ts index a9844e1c5f1..53b161876f0 100644 --- a/apps/desktop/src/renderer/stores/tabs/utils.ts +++ b/apps/desktop/src/renderer/stores/tabs/utils.ts @@ -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 @@ -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, }; diff --git a/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.test.ts b/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.test.ts new file mode 100644 index 00000000000..0e958974b26 --- /dev/null +++ b/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.test.ts @@ -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)"); + }); +}); diff --git a/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.ts b/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.ts new file mode 100644 index 00000000000..481e21594ad --- /dev/null +++ b/apps/desktop/src/renderer/stores/tabs/utils/terminal-naming.ts @@ -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); +};