From ab01a7e83afd60cb36bc161a9b508bf21f6967eb Mon Sep 17 00:00:00 2001 From: abose Date: Mon, 28 Sep 2026 16:15:05 +0530 Subject: [PATCH] feat(terminal): name the tabs, and let the user rename them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every tab carried the same terminal icon, which told them apart not at all: open three shells and the strip showed three identical glyphs. The name each tab already knew was hidden in the narrow strip, and shown only when the panel was wide enough. So the icon gives up its slot — the strip widens a little and always shows the name. The name is the running process, which says nothing while several shells sit idle at a prompt, all reading "zsh". A pencil on hover lets the user name a tab themselves; the name shows in italics so it reads as theirs, and emptying the field hands the tab back to the process label. Names are kept globally, so one means the same thing wherever the user is working, with no project switching to reason about. They are stored as a list in tab order: terminal ids are handed out afresh on every run and again on every restart, so nothing identifies a tab across them, while a list is exactly what says how many tabs to bring back and what to call them. Opening an empty panel restores the lot at once — the panel is usually shut when a window starts, and bringing tabs back one press of + at a time would not be bringing them back at all. Restarting the terminals for a new project carries the names onto the replacements, and closing a named tab drops it from the list, which is how the user says not to bring it back. Nothing but the tabs and their names returns: the shells are new, and with nothing named the panel opens the single terminal it always did. Three details the implementation turns on. The pencil's click is bound to the pencil rather than delegated, so it runs before the row's own handler and can stop it there: let it through and the row activates the terminal, which takes focus straight back off the field — the same reason a double click could never work. A refresh will not rebuild the rows while a field is open, because emptying the list tears the field out, the removal fires blur, and blur commits, landing a half-typed name nobody confirmed. And the tick that finishes an edit commits on mousedown, because a click would let the field blur and finish first, taking the button away between the press and the release. Hovering trades the cwd for the controls rather than reserving room for them, which leaves the name more space, not less, and spares every future control the margin arithmetic that went stale the moment a second one appeared. The field leaves its background and text colour to the theme so it reads like every other input, and sets all four margins, the theme's own bottom margin for form inputs having pushed it off the row's centre. --- src/extensionsIntegrated/Terminal/main.js | 126 ++++++++++++++++------ src/nls/root/strings.js | 1 + src/styles/Extn-Terminal.less | 33 +++++- test/spec/Terminal-integ-test.js | 126 ++++++++++++++++++++-- 4 files changed, 246 insertions(+), 40 deletions(-) diff --git a/src/extensionsIntegrated/Terminal/main.js b/src/extensionsIntegrated/Terminal/main.js index be2b98e776..ea7bae6480 100644 --- a/src/extensionsIntegrated/Terminal/main.js +++ b/src/extensionsIntegrated/Terminal/main.js @@ -356,8 +356,6 @@ define(function (require, exports, module) { const instance = new TerminalInstance(nodeConnector, shell, cwd); // Project ownership is independent of directories the user visits in the shell. instance.projectPath = projectPath || (projectRoot ? projectRoot.fullPath : null); - // a name given to this slot in an earlier run comes back with it - instance.customName = _loadTabNames()[instance.id] || null; // Set up callbacks instance.onTitleChanged = _onTerminalTitleChanged; @@ -490,15 +488,26 @@ define(function (require, exports, module) { if (!currentRoot || currentRoot.fullPath !== path) { return; } - const profiles = terminalInstances.map(inst => inst.shellProfile); + // A restart replaces every instance, and the new ones are issued + // fresh ids, so a name looked up by id would not find them. Carry + // the names across with the shells: the tabs are the same tabs to + // the user, only pointed at the new project. + const carried = terminalInstances.map(inst => ({ + profile: inst.shellProfile, + customName: inst.customName + })); const activeIndex = terminalInstances.findIndex(inst => inst.id === activeTerminalId); await _disposeAllAsync(); activeTerminalId = null; _updateFlyout(); const replacements = []; - for (const profile of profiles) { - replacements.push(await _createNewTerminalWithShell(profile, path, path)); + for (const item of carried) { + const replacement = await _createNewTerminalWithShell(item.profile, path, path); + replacement.customName = item.customName; + replacements.push(replacement); } + // the names were set after each tab rendered, so draw them again + _updateFlyout(); if (replacements[activeIndex]) { _activateTerminal(replacements[activeIndex].id); } @@ -573,6 +582,8 @@ define(function (require, exports, module) { instance.dispose(); terminalInstances.splice(idx, 1); delete processInfo[id]; + // closing a named tab is how the user says not to bring it back + _saveTabNames(); if ($contentArea.find(".terminal-project-banner").length) { _showProjectBanner(); } @@ -699,38 +710,47 @@ define(function (require, exports, module) { } /** - * Names the user has given terminal tabs, kept globally rather than per - * project so a name means the same thing wherever they are working. + * The names the user has given terminal tabs, in strip order, kept globally + * rather than per project so a name means the same thing wherever they are + * working. * - * Keyed by terminal id, which counts up from term_1 in creation order and - * starts over each run: a name therefore belongs to a tab's place in the - * strip rather than to one shell, and the second terminal opened next run - * gets the name the second terminal had. That is as close to "remember my - * terminals" as this can get until the panel restores a session at all — - * it recreates nothing on boot today, so there is no terminal to hand a - * name back to. Names are kept when a tab closes for the same reason: - * dropping them would leave nothing to remember by the next run. + * A list rather than a map off terminal ids: ids are handed out afresh each + * run and again on every restart, so nothing identifies a shell across + * them. What the user is naming is the tab, and the list is what says how + * many tabs to bring back and what to call them. * - * @return {Object} id -> name, empty when nothing has been named + * @return {Array} names in order, empty when nothing has been named */ function _loadTabNames() { const stored = StateManager.get(STATE_TAB_NAMES); - return (stored && typeof stored === "object") ? stored : {}; + if (Array.isArray(stored)) { + return stored.filter(function (n) { return typeof n === "string" && n; }); + } + // an earlier build kept these against terminal ids; order them by the + // number in the id rather than the text of it, or term_10 sorts before + // term_2 and the tabs come back shuffled + if (stored && typeof stored === "object") { + const idOrder = function (k) { + const digits = /(\d+)/.exec(k); + return digits ? parseInt(digits[1], 10) : 0; + }; + return Object.keys(stored) + .sort(function (a, b) { return idOrder(a) - idOrder(b); }) + .map(function (k) { return stored[k]; }) + .filter(Boolean); + } + return []; } /** - * Record or forget one tab's name. - * @param {string} id - terminal id - * @param {?string} name - the name, or null to go back to the process label + * Write the current tabs' names back, so the set that returns next run is + * the set on screen now. Unnamed tabs are left out: naming one is what asks + * for it to come back. */ - function _saveTabName(id, name) { - const names = _loadTabNames(); - if (name) { - names[id] = name; - } else { - delete names[id]; - } - StateManager.set(STATE_TAB_NAMES, names); + function _saveTabNames() { + StateManager.set(STATE_TAB_NAMES, terminalInstances + .map(function (inst) { return inst.customName; }) + .filter(Boolean)); } /** @@ -756,9 +776,12 @@ define(function (require, exports, module) { } const $input = $('') .val(inst.customName || $title.text().trim()); + const $done = $(''); $item.addClass("renaming"); renamingTerminalId = inst.id; $title.hide().after($input); + $input.after($done); $input.trigger("focus").trigger("select"); let settled = false; @@ -770,11 +793,12 @@ define(function (require, exports, module) { const name = $input.val().trim(); // an emptied field means "go back to naming it after the process" inst.customName = name || null; - _saveTabName(inst.id, inst.customName); + _saveTabNames(); Metrics.countEvent(Metrics.EVENT_TYPE.TERMINAL, "rename", name ? "set" : "clear"); } $input.remove(); + $done.remove(); $title.show(); $item.removeClass("renaming"); _updateFlyout(); @@ -788,6 +812,15 @@ define(function (require, exports, module) { $input.on("blur", function () { finish(true); }); // a double click inside the field shouldn't re-enter renaming $input.on("dblclick click", function (e) { e.stopPropagation(); }); + // Committed on mousedown, before the field can lose focus: letting the + // blur land first would finish the edit and take this button away + // between press and release, so the click would never arrive. + $done.on("mousedown", function (e) { + e.preventDefault(); + e.stopPropagation(); + finish(true); + }); + $done.on("click", function (e) { e.stopPropagation(); }); } function _updateFlyout() { @@ -878,6 +911,39 @@ define(function (require, exports, module) { * @param {string} [options.shellCommand] - A shell command to execute in a new terminal. * When provided, always creates a fresh terminal and types the command into it. */ + /** + * Open the tabs the user named last time, all of them, in the order they + * were in. + * + * The panel is often shut when a window starts, so this runs the first time + * it opens with nothing in it. Bringing the tabs back one at a time as the + * user presses + would not be bringing them back at all — the point of + * naming a terminal is that it is waiting where it was left. + * + * Shells are new: nothing of what ran in them is restored, only the tabs + * and what they are called. With nothing saved this is the plain single + * terminal the panel has always opened with. + */ + async function _restoreSavedTerminals() { + const names = _loadTabNames(); + if (!names.length) { + await _createNewTerminal(); + return; + } + for (const name of names) { + await _createNewTerminal(); + const restored = terminalInstances[terminalInstances.length - 1]; + if (restored) { + restored.customName = name; + } + } + // each tab rendered before its name was put back + _updateFlyout(); + if (terminalInstances.length) { + _activateTerminal(terminalInstances[0].id); + } + } + async function _showTerminal(options) { if (options && options.shellCommand) { await _createNewTerminal(); @@ -893,7 +959,7 @@ define(function (require, exports, module) { return; } if (terminalInstances.length === 0) { - await _createNewTerminal(); + await _restoreSavedTerminals(); return; } const active = _getActiveTerminal(); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index cd9b8c7be6..f3af4ffa30 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -2229,6 +2229,7 @@ define({ "ERROR_SAVE_FIRST": "Save the document first!", "ERROR_TERMINAL_NOT_FOUND": "Terminal was not found for your OS, you can define a custom Terminal command in the settings", "TERMINAL_RENAME_TAB": "Rename this terminal", + "TERMINAL_RENAME_DONE": "Done", "TERMINAL_CLOSE_CONFIRM_TITLE": "Active Process Running", "TERMINAL_CLOSE_CONFIRM_MSG": "Terminal has an active process running: {0}.
Are you sure you want to close it?", "TERMINAL_CLOSE_SINGLE_TITLE": "Close Terminal?", diff --git a/src/styles/Extn-Terminal.less b/src/styles/Extn-Terminal.less index 0db9f37f51..1408c18094 100644 --- a/src/styles/Extn-Terminal.less +++ b/src/styles/Extn-Terminal.less @@ -291,13 +291,16 @@ input[type="text"].terminal-flyout-rename { flex: 1; min-width: 0; height: 20px; - margin-left: 6px; - margin-right: 18px; + /* all four sides, because the theme gives form inputs a bottom margin of + their own and that pushes the field off the row's centre line */ + margin: 0 22px 0 6px; padding: 0 4px; font-size: 12px; line-height: 18px; - color: var(--terminal-tab-active-text); - background: rgba(0, 0, 0, 0.35); + /* background and text colour are left to the theme's own input styling, so + this field reads like every other one — white on a light theme rather + than a dark patch on a light strip. Only the border is ours, to mark the + row as being edited. */ border: 1px solid #007acc; border-radius: 2px; outline: none; @@ -310,6 +313,28 @@ input[type="text"].terminal-flyout-rename { display: none; } +/* Confirm button, in the slot the pencil and close vacate while editing */ +.terminal-flyout-rename-done { + position: absolute; + right: 0; + top: 0; + bottom: 0; + width: 20px; + display: flex; + align-items: center; + justify-content: center; + font-size: 10px; + color: var(--terminal-tab-text); + background: transparent; + z-index: 2; + cursor: pointer; +} + +.terminal-flyout-rename-done:hover { + color: var(--terminal-tab-active-text); + background: rgba(255, 255, 255, 0.1); +} + /* ─── Flyout bottom actions ─── */ .terminal-flyout-actions { diff --git a/test/spec/Terminal-integ-test.js b/test/spec/Terminal-integ-test.js index 7a054a08a7..1e64b1a1c0 100644 --- a/test/spec/Terminal-integ-test.js +++ b/test/spec/Terminal-integ-test.js @@ -652,11 +652,10 @@ define(function (require, exports, module) { }, "terminal panel to close", 5000); } - it("should rename a tab and remember the name for its slot", async function () { + it("should rename a tab and remember it for the next run", async function () { await openOneTerminal(); const autoLabel = tabLabel(); - const id = $activeTab().attr("data-terminal-id"); commitRename(startRename(), "build", "Enter"); await awaitsFor(function () { @@ -665,15 +664,16 @@ define(function (require, exports, module) { // marked as the user's own, not a process we detected expect($activeTab().hasClass("renamed")).toBeTrue(); expect(tabLabel()).not.toBe(autoLabel); - // and written through, so the slot keeps it for the next run - expect(StateManager.get(STATE_TAB_NAMES)[id]).toBe("build"); + // written through as a list, which is what says how many tabs + // to bring back next time and what to call them + expect(StateManager.get(STATE_TAB_NAMES)).toEqual(["build"]); + commitRename(startRename(), "", "Enter"); await closePanel(); }); it("should return a tab to its process label when the name is cleared", async function () { await openOneTerminal(); - const id = $activeTab().attr("data-terminal-id"); commitRename(startRename(), "build", "Enter"); await awaitsFor(function () { return tabLabel() === "build"; @@ -686,7 +686,8 @@ define(function (require, exports, module) { }, "tab to fall back to its process label", 5000); expect($activeTab().hasClass("renamed")).toBeFalse(); - expect(StateManager.get(STATE_TAB_NAMES)[id]).toBeUndefined(); + // and drops out of what comes back next run + expect(StateManager.get(STATE_TAB_NAMES)).toEqual([]); await closePanel(); }); @@ -761,6 +762,90 @@ define(function (require, exports, module) { return !testWindow.$("#terminal-panel").is(":visible"); }, "terminal panel to close", 5000); }); + + it("should reopen every saved tab at once when the panel opens", async function () { + const termModule = testWindow.brackets.getModule( + "extensionsIntegrated/Terminal/main" + ); + StateManager.set(STATE_TAB_NAMES, ["build", "server", "tests"]); + await termModule._disposeAll(); + WorkspaceManager.getPanelForID(PANEL_ID).hide(); + + // The panel is usually shut when a window starts. Opening it is + // what brings the tabs back — all of them, not one per press of + // the new-terminal button. + await openTerminal(); + await awaitsFor(function () { + return getTerminalCount() === 3; + }, "every saved tab to come back", 20000); + + const names = testWindow.$(".terminal-flyout-title").map(function () { + return testWindow.$(this).text(); + }).get(); + expect(names).toEqual(["build", "server", "tests"]); + expect(testWindow.$(".terminal-flyout-item.renamed").length).toBe(3); + expect(testWindow.$(".terminal-flyout-item.active").index()).toBe(0); + + StateManager.set(STATE_TAB_NAMES, []); + await termModule._disposeAll(); + WorkspaceManager.getPanelForID(PANEL_ID).hide(); + }, 40000); + + it("should open a single terminal when nothing has been named", async function () { + const termModule = testWindow.brackets.getModule( + "extensionsIntegrated/Terminal/main" + ); + StateManager.set(STATE_TAB_NAMES, []); + await termModule._disposeAll(); + WorkspaceManager.getPanelForID(PANEL_ID).hide(); + + await openTerminal(); + await awaitsFor(function () { + return getTerminalCount() === 1; + }, "the usual single terminal", 10000); + expect(testWindow.$(".terminal-flyout-item.renamed").length).toBe(0); + + await closePanel(); + }); + + it("should place the rename field on the row's centre line", async function () { + await openOneTerminal(); + startRename(); + const $input = testWindow.$(".terminal-flyout-rename"); + const row = $activeTab()[0].getBoundingClientRect(); + const field = $input[0].getBoundingClientRect(); + + // the theme gives form inputs a bottom margin of their own, + // which pushed the field off centre until it was zeroed + const offset = (field.top + field.height / 2) + - (row.top + row.height / 2); + expect(Math.abs(offset)).toBeLessThan(2); + expect(field.height).toBeLessThan(row.height); + expect(field.top).not.toBeLessThan(row.top); + expect(field.bottom).not.toBeGreaterThan(row.bottom); + + commitRename($input, "", "Escape"); + await closePanel(); + }); + + it("should confirm the edit from the tick button", async function () { + await openOneTerminal(); + const $input = startRename(); + $input.val("via-tick"); + + // mousedown, not click: a click would let the field blur and + // finish first, taking the button away before the press lands + testWindow.$(".terminal-flyout-rename-done").trigger("mousedown"); + await awaitsFor(function () { + return testWindow.$(".terminal-flyout-rename").length === 0; + }, "rename field to close", 5000); + + expect(tabLabel()).toBe("via-tick"); + expect($activeTab().hasClass("renamed")).toBeTrue(); + + commitRename(startRename(), "", "Enter"); + await closePanel(); + }); }); describe("Project-switch banner", function () { @@ -900,6 +985,35 @@ define(function (require, exports, module) { expect(first.isAlive).toBeTrue(); }, 30000); + it("keeps tab names when the terminals restart in the new project", async function () { + const first = await openReadyTerminal(); + + // name it, the way the pencil does + testWindow.$(".terminal-flyout-item.active .terminal-flyout-edit").click(); + const $field = testWindow.$(".terminal-flyout-rename"); + $field.val("build"); + $field.trigger(testWindow.$.Event("keydown", {key: "Enter"})); + await awaitsFor(function () { + return testWindow.$(".terminal-flyout-item.active .terminal-flyout-title") + .text() === "build"; + }, "tab to take the typed name", 5000); + + // A restart replaces the instance, and the replacement is issued + // a fresh id — the name has to travel with it rather than be + // looked up by an id that no longer exists. + await SpecRunnerUtils.loadProjectInTestWindow(secondProjectPath); + testWindow.$(".terminal-project-restart").click(); + await awaitsFor(function () { + const active = termModule._getActiveTerminal(); + return getTerminalCount() === 1 && active && active.isAlive + && active.id !== first.id; + }, "terminal to be replaced by the restart", 15000); + + expect(testWindow.$(".terminal-flyout-item.active .terminal-flyout-title").text()) + .toBe("build"); + expect(testWindow.$(".terminal-flyout-item.active").hasClass("renamed")).toBeTrue(); + }); + it("restarts every tab in the new project and preserves its shell and selection", async function () { const first = await openReadyTerminal(); const ShellProfiles = testWindow.brackets.getModule("extensionsIntegrated/Terminal/ShellProfiles");