Skip to content
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.

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

Allow changing terminal shell in Teleport Connect #45152

Merged
merged 39 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
257c965
Get available shells for the system and store them in `RuntimeSettings`
gzdunek Aug 6, 2024
e5fd3b7
Add a config option for `terminal.shell`
gzdunek Aug 6, 2024
74fb5cf
Add `shellId` to terminal documents and pty commands
gzdunek Aug 6, 2024
431b0f7
Pass the `shellId` from pty command to pty process options (and resol…
gzdunek Aug 6, 2024
1fbed56
Allow changing the active and default shell from the context menu
gzdunek Aug 6, 2024
74bdc2b
Show active shell in the tab name, drop "Terminal" from the name on W…
gzdunek Aug 6, 2024
baedfa8
Fix resolving shell env for `csh`
gzdunek Aug 6, 2024
03d7b16
Do not read unix shells from `/etc/shells`, add a config property for…
gzdunek Aug 13, 2024
5e3e407
Add a whitelist of config properties that can be modified from the re…
gzdunek Aug 13, 2024
58b78f4
Allow selecting custom shell
gzdunek Aug 13, 2024
563541f
Return back the actually opened shell and creation status
gzdunek Aug 13, 2024
ffaf668
Pass variables to WSL via WSLENV
gzdunek Aug 13, 2024
9b017fd
Merge branch 'master' into gzdunek/shell-switcher
gzdunek Aug 13, 2024
e767729
Do not change the default shell when selecting a custom shell
gzdunek Aug 13, 2024
ac675c9
Show profile switcher only when there is more than one shell to choos…
gzdunek Aug 14, 2024
a040671
Show shell name in the "Default Shell" context menu as sublabel
gzdunek Aug 14, 2024
531d39e
Remove comment about sync setup of `setUpDeepLinks`
gzdunek Aug 19, 2024
3f6a561
Replace `MainTabContextMenuOptions` type
gzdunek Aug 19, 2024
11c238c
Merge branch 'master' into gzdunek/shell-switcher
gzdunek Aug 19, 2024
5fb0680
Improve types in `subscribeToTabContextMenuEvent`
gzdunek Aug 21, 2024
263e0dd
Use early return
gzdunek Aug 21, 2024
230a1bc
Fix condition in the schema for `terminal.shell`
gzdunek Aug 21, 2024
cdab839
Expand the comment for `preventAutoPromiseResolveOnMenuClose`
gzdunek Aug 21, 2024
8b59b26
Render available shells without an array
gzdunek Aug 22, 2024
28a719c
`openedShell` -> `shell`
gzdunek Aug 22, 2024
28ba4a4
Add tests for resolving shell
gzdunek Aug 22, 2024
9b5b4b2
Remove `PtyOptions`
gzdunek Aug 22, 2024
6cbd5ea
Preserve user defined WSLENV
gzdunek Aug 22, 2024
0efbc78
Remove the check for `fs.access(customShellPath)`
gzdunek Aug 22, 2024
1f14c0f
Mock `process.env` instead of mutating it
gzdunek Aug 22, 2024
e38f2c4
Fix remaining `openedShell` -> `shell`
gzdunek Aug 22, 2024
3f042d1
Fix `PATH` issue on Windows
gzdunek Aug 22, 2024
52f0485
Remove updating a document with the default shell in `useDocumentTerm…
gzdunek Aug 22, 2024
cc84c82
Add `canDocChangeShell`
gzdunek Aug 22, 2024
6c2af71
Omit the default shell name on Linux too
gzdunek Aug 22, 2024
0ec349b
Convert `buildPtyOptions` parameters to object, pass `process.env` fr…
gzdunek Aug 22, 2024
870fe43
Revert "Remove updating a document with the default shell in `useDocu…
gzdunek Aug 22, 2024
d45ef0d
Improve comments
gzdunek Aug 23, 2024
ef3f622
Throw when the document doesn't exist
gzdunek Aug 23, 2024
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
9 changes: 3 additions & 6 deletions web/packages/teleterm/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ if (app.requestSingleInstanceLock()) {
app.exit(1);
}

function initializeApp(): void {
async function initializeApp(): Promise<void> {
gzdunek marked this conversation as resolved.
Show resolved Hide resolved
updateSessionDataPath();
let devRelaunchScheduled = false;
const settings = getRuntimeSettings();
const settings = await getRuntimeSettings();
const logger = initMainLogger(settings);
logger.info(`Starting ${app.getName()} version ${app.getVersion()}`);
const {
Expand All @@ -76,7 +76,7 @@ function initializeApp(): void {
const configService = createConfigService({
configFile: configFileStorage,
jsonSchemaFile: configJsonSchemaFileStorage,
platform: settings.platform,
settings,
});

nativeTheme.themeSource = configService.get('theme').value;
Expand Down Expand Up @@ -150,9 +150,6 @@ function initializeApp(): void {
// Since setUpDeepLinks adds another listener for second-instance, it's important to call it after
// the listener which calls windowsManager.focusWindow. This way the focus will be brought to the
// window before processing the listener for deep links.
//
// The setup must be done synchronously when starting the app, otherwise the listeners won't get
// triggered on macOS if the app is not already running when the user opens a deep link.
setUpDeepLinks(logger, windowsManager, settings);

const rootClusterProxyHostAllowList = new Set<string>();
Expand Down
162 changes: 144 additions & 18 deletions web/packages/teleterm/src/mainProcess/contextMenus/tabContextMenu.ts
ravicious marked this conversation as resolved.
Show resolved Hide resolved
ravicious marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -21,61 +21,177 @@ import {
ipcRenderer,
Menu,
MenuItemConstructorOptions,
dialog,
} from 'electron';

import { ConfigService } from 'teleterm/services/config';
import { Shell, makeCustomShellFromPath } from 'teleterm/mainProcess/shell';

import {
TabContextMenuEventChannel,
TabContextMenuEventType,
TabContextMenuOptions,
} from '../types';

type MainTabContextMenuOptions = Pick<TabContextMenuOptions, 'documentKind'>;
import type { Document } from 'teleterm/ui/services/workspacesService';

type MainTabContextMenuOptions = {
document: Document;
};

type TabContextMenuEvent =
| {
event: TabContextMenuEventType.ReopenPtyInShell;
item: Shell;
}
| {
event:
| TabContextMenuEventType.Close
| TabContextMenuEventType.CloseOthers
| TabContextMenuEventType.CloseToRight
| TabContextMenuEventType.DuplicatePty;
};

export function subscribeToTabContextMenuEvent(): void {
export function subscribeToTabContextMenuEvent(
shells: Shell[],
configService: ConfigService
): void {
ipcMain.handle(
TabContextMenuEventChannel,
(event, options: MainTabContextMenuOptions) => {
return new Promise(resolve => {
return new Promise<TabContextMenuEvent>(resolve => {
let preventAutoPromiseResolveOnMenuClose = false;
gzdunek marked this conversation as resolved.
Show resolved Hide resolved

function getCommonTemplate(): MenuItemConstructorOptions[] {
return [
{
label: 'Close',
click: () => resolve(TabContextMenuEventType.Close),
click: () => resolve({ event: TabContextMenuEventType.Close }),
},
{
label: 'Close Others',
click: () => resolve(TabContextMenuEventType.CloseOthers),
click: () =>
resolve({ event: TabContextMenuEventType.CloseOthers }),
},
{
label: 'Close to the Right',
click: () => resolve(TabContextMenuEventType.CloseToRight),
click: () =>
resolve({ event: TabContextMenuEventType.CloseToRight }),
},
];
}

function getPtyTemplate(): MenuItemConstructorOptions[] {
if (
options.documentKind === 'doc.terminal_shell' ||
options.documentKind === 'doc.terminal_tsh_node'
options.document.kind === 'doc.terminal_shell' ||
options.document.kind === 'doc.terminal_tsh_node'
) {
return [
{
type: 'separator',
},
{
label: 'Duplicate Tab',
click: () => resolve(TabContextMenuEventType.DuplicatePty),
click: () =>
resolve({ event: TabContextMenuEventType.DuplicatePty }),
},
];
}
}

function getShellTemplate(): MenuItemConstructorOptions[] {
const doc = options.document;
const canChangeShell =
doc.kind === 'doc.terminal_shell' ||
doc.kind === 'doc.gateway_kube';
if (!canChangeShell) {
return;
}
const activeShellId = doc.shellId;
const defaultShellId = configService.get('terminal.shell').value;
const customShellPath = configService.get(
'terminal.customShell'
).value;
const customShell =
customShellPath && makeCustomShellFromPath(customShellPath);
const shellsWithCustom = [...shells, customShell].filter(Boolean);
const isMoreThanOneShell = shellsWithCustom.length > 1;
return [
{
type: 'separator',
},
...shellsWithCustom.map(shell => ({
label: shell.friendlyName,
id: shell.id,
type: 'radio' as const,
visible: isMoreThanOneShell,
checked: shell.id === activeShellId,
click: () => {
// Do nothing when the shell doesn't change.
if (shell.id === activeShellId) {
return;
}
resolve({
event: TabContextMenuEventType.ReopenPtyInShell,
item: shell,
});
},
})),
{
label: customShell
? `Change Custom Shell (${customShell.friendlyName})…`
: 'Select Custom Shell…',
click: async () => {
// By default, when the popup menu is closed, the promise is
// resolved (popup.callback).
// Here we need to prevent this behavior to wait for the file
// to be selected.
// A more universal way of handling this problem:
// https://github.com/gravitational/teleport/pull/45152#discussion_r1723314524
preventAutoPromiseResolveOnMenuClose = true;
const { filePaths, canceled } = await dialog.showOpenDialog({
properties: ['openFile'],
defaultPath: customShell.binPath,
});
if (canceled) {
resolve(undefined);
return;
}
const file = filePaths[0];
configService.set('terminal.customShell', file);
resolve({
event: TabContextMenuEventType.ReopenPtyInShell,
item: makeCustomShellFromPath(file),
});
},
},
{
label: 'Default Shell',
visible: isMoreThanOneShell,
type: 'submenu',
sublabel:
shellsWithCustom.find(s => defaultShellId === s.id)
?.friendlyName || defaultShellId,
submenu: [
...shellsWithCustom.map(shell => ({
label: shell.friendlyName,
id: shell.id,
checked: shell.id === defaultShellId,
type: 'radio' as const,
click: () => {
configService.set('terminal.shell', shell.id);
gzdunek marked this conversation as resolved.
Show resolved Hide resolved
resolve(undefined);
},
})),
],
},
];
}

Menu.buildFromTemplate(
[getCommonTemplate(), getPtyTemplate()]
[getCommonTemplate(), getPtyTemplate(), getShellTemplate()]
.filter(Boolean)
.flatMap(template => template)
).popup({
callback: () => resolve(undefined),
callback: () =>
!preventAutoPromiseResolveOnMenuClose && resolve(undefined),
});
});
}
Expand All @@ -86,13 +202,19 @@ export async function openTabContextMenu(
options: TabContextMenuOptions
): Promise<void> {
const mainOptions: MainTabContextMenuOptions = {
documentKind: options.documentKind,
document: options.document,
};
const eventType = await ipcRenderer.invoke(
const response = (await ipcRenderer.invoke(
TabContextMenuEventChannel,
mainOptions
);
switch (eventType) {
)) as TabContextMenuEvent | undefined;
// Undefined when the menu gets closed without clicking on any action.
if (!response) {
return;
}
const { event } = response;

switch (event) {
case TabContextMenuEventType.Close:
return options.onClose();
case TabContextMenuEventType.CloseOthers:
Expand All @@ -101,5 +223,9 @@ export async function openTabContextMenu(
return options.onCloseToRight();
case TabContextMenuEventType.DuplicatePty:
return options.onDuplicatePty();
case TabContextMenuEventType.ReopenPtyInShell:
return options.onReopenPtyInShell(response.item);
default:
event satisfies never;
}
}
7 changes: 5 additions & 2 deletions web/packages/teleterm/src/mainProcess/fixtures/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class MockMainProcessClient implements MainProcessClient {
this.configService = createConfigService({
configFile: createMockFileStorage(),
jsonSchemaFile: createMockFileStorage(),
platform: this.getRuntimeSettings().platform,
settings: this.getRuntimeSettings(),
});
}

Expand Down Expand Up @@ -150,7 +150,10 @@ export const makeRuntimeSettings = (
certsDir: '',
kubeConfigsDir: '',
logsDir: '',
defaultShell: '',
defaultOsShellId: 'zsh',
availableShells: [
{ id: 'zsh', friendlyName: 'zsh', binPath: '/bin/zsh', binName: 'zsh' },
],
tshd: {
requestedNetworkAddress: '',
binaryPath: '',
Expand Down
5 changes: 4 additions & 1 deletion web/packages/teleterm/src/mainProcess/mainProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,10 @@ export default class MainProcess {
);

subscribeToTerminalContextMenuEvent(this.configService);
subscribeToTabContextMenuEvent();
subscribeToTabContextMenuEvent(
this.settings.availableShells,
this.configService
);
subscribeToConfigServiceEvents(this.configService);
subscribeToFileStorageEvents(this.appStateFileStorage);
}
Expand Down
32 changes: 5 additions & 27 deletions web/packages/teleterm/src/mainProcess/runtimeSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ import path from 'path';

import { app } from 'electron';

import Logger from 'teleterm/logger';

import { GrpcServerAddresses, RuntimeSettings } from './types';
import { loadInstallationId } from './loadInstallationId';
import { getAvailableShells, getDefaultShell } from './shell';

const { argv, env } = process;

Expand Down Expand Up @@ -57,7 +56,7 @@ const insecure =
// flag one level down.
(dev && !!env.CONNECT_INSECURE);

export function getRuntimeSettings(): RuntimeSettings {
export async function getRuntimeSettings(): Promise<RuntimeSettings> {
const userDataDir = app.getPath('userData');
const sessionDataDir = app.getPath('sessionData');
const tempDataDir = app.getPath('temp');
Expand Down Expand Up @@ -98,6 +97,7 @@ export function getRuntimeSettings(): RuntimeSettings {
//
// A workaround is to read the version from `process.env.npm_package_version`.
const appVersion = dev ? process.env.npm_package_version : app.getVersion();
const availableShells = await getAvailableShells();

return {
dev,
Expand All @@ -112,7 +112,8 @@ export function getRuntimeSettings(): RuntimeSettings {
binDir,
agentBinaryPath: path.resolve(sessionDataDir, 'teleport', 'teleport'),
certsDir: getCertsDir(),
defaultShell: getDefaultShell(),
availableShells,
defaultOsShellId: getDefaultShell(availableShells),
kubeConfigsDir,
logsDir,
platform: process.platform,
Expand Down Expand Up @@ -203,29 +204,6 @@ export function getAssetPath(...paths: string[]): string {
return path.join(RESOURCES_PATH, 'assets', ...paths);
}

function getDefaultShell(): string {
const logger = new Logger();
switch (process.platform) {
case 'linux':
case 'darwin': {
const fallbackShell = 'bash';
const { shell } = os.userInfo();

if (!shell) {
logger.error(
`Failed to read ${process.platform} platform default shell, using fallback: ${fallbackShell}.\n`
);

return fallbackShell;
}

return shell;
}
case 'win32':
return 'powershell.exe';
}
}

/**
* Describes what addresses the gRPC servers should attempt to obtain on app startup.
*/
Expand Down
Loading
Loading