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");