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

Launch Native REPL using terminal link #24734

Merged
merged 22 commits into from
Jan 23, 2025
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
5 changes: 5 additions & 0 deletions python_files/pythonrc.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,8 @@ def __str__(self):

if sys.platform != "win32" and (not is_wsl) and use_shell_integration:
sys.ps1 = PS1()

if sys.platform == "darwin":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it weird if only part of this string is fixed to create a link with l10n? Will that translate only half the command?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean tooltip: l10n.t('Launch VS Code Native REPL'), part?
I think the the tooltip would only show Launch VS Code Native REPL

print("Cmd click to launch VS Code Native REPL")
else:
print("Ctrl click to launch VS Code Native REPL")
20 changes: 20 additions & 0 deletions python_files/tests/test_shell_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,23 @@ def test_excepthook_call():

hooks.my_excepthook("mock_type", "mock_value", "mock_traceback")
mock_excepthook.assert_called_once_with("mock_type", "mock_value", "mock_traceback")


if sys.platform == "darwin":

def test_print_statement_darwin(monkeypatch):
importlib.reload(pythonrc)
with monkeypatch.context() as m:
m.setattr("builtins.print", Mock())
importlib.reload(sys.modules["pythonrc"])
print.assert_any_call("Cmd click to launch VS Code Native REPL")


if sys.platform == "win32":

def test_print_statement_non_darwin(monkeypatch):
importlib.reload(pythonrc)
with monkeypatch.context() as m:
m.setattr("builtins.print", Mock())
importlib.reload(sys.modules["pythonrc"])
print.assert_any_call("Ctrl click to launch VS Code Native REPL")
1 change: 1 addition & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export namespace AttachProcess {

export namespace Repl {
export const disableSmartSend = l10n.t('Disable Smart Send');
export const launchNativeRepl = l10n.t('Launch VS Code Native REPL');
}
export namespace Pylance {
export const remindMeLater = l10n.t('Remind me later');
Expand Down
5 changes: 5 additions & 0 deletions src/client/common/vscodeApis/windowApis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
TerminalShellExecutionStartEvent,
LogOutputChannel,
OutputChannel,
TerminalLinkProvider,
} from 'vscode';
import { createDeferred, Deferred } from '../utils/async';
import { Resource } from '../types';
Expand Down Expand Up @@ -258,3 +259,7 @@ export function createOutputChannel(name: string, languageId?: string): OutputCh
export function createLogOutputChannel(name: string, options: { log: true }): LogOutputChannel {
return window.createOutputChannel(name, options);
}

export function registerTerminalLinkProvider(provider: TerminalLinkProvider): Disposable {
return window.registerTerminalLinkProvider(provider);
}
2 changes: 2 additions & 0 deletions src/client/extensionActivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import { registerReplCommands, registerReplExecuteOnEnter, registerStartNativeRe
import { registerTriggerForTerminalREPL } from './terminals/codeExecution/terminalReplWatcher';
import { registerPythonStartup } from './terminals/pythonStartup';
import { registerPixiFeatures } from './pythonEnvironments/common/environmentManagers/pixi';
import { registerCustomTerminalLinkProvider } from './terminals/pythonStartupLinkProvider';

export async function activateComponents(
// `ext` is passed to any extra activation funcs.
Expand Down Expand Up @@ -115,6 +116,7 @@ export function activateFeatures(ext: ExtensionState, _components: Components):
registerStartNativeReplCommand(ext.disposables, interpreterService);
registerReplCommands(ext.disposables, interpreterService, executionHelper, commandManager);
registerReplExecuteOnEnter(ext.disposables, interpreterService, commandManager);
registerCustomTerminalLinkProvider(ext.disposables);
}

/// //////////////////////////
Expand Down
44 changes: 44 additions & 0 deletions src/client/terminals/pythonStartupLinkProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/* eslint-disable class-methods-use-this */
import {
CancellationToken,
Disposable,
ProviderResult,
TerminalLink,
TerminalLinkContext,
TerminalLinkProvider,
} from 'vscode';
import { executeCommand } from '../common/vscodeApis/commandApis';
import { registerTerminalLinkProvider } from '../common/vscodeApis/windowApis';
import { Repl } from '../common/utils/localize';

interface CustomTerminalLink extends TerminalLink {
command: string;
}

export class CustomTerminalLinkProvider implements TerminalLinkProvider<CustomTerminalLink> {
provideTerminalLinks(
context: TerminalLinkContext,
_token: CancellationToken,
): ProviderResult<CustomTerminalLink[]> {
const links: CustomTerminalLink[] = [];
const expectedNativeLink = 'VS Code Native REPL';

if (context.line.includes(expectedNativeLink)) {
links.push({
startIndex: context.line.indexOf(expectedNativeLink),
length: expectedNativeLink.length,
tooltip: Repl.launchNativeRepl,
command: 'python.startNativeREPL',
});
}
return links;
}

async handleTerminalLink(link: CustomTerminalLink): Promise<void> {
await executeCommand(link.command);
}
}

export function registerCustomTerminalLinkProvider(disposables: Disposable[]): void {
disposables.push(registerTerminalLinkProvider(new CustomTerminalLinkProvider()));
}
76 changes: 74 additions & 2 deletions src/test/terminals/shellIntegration/pythonStartup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,23 @@

import * as sinon from 'sinon';
import * as TypeMoq from 'typemoq';
import { GlobalEnvironmentVariableCollection, Uri, WorkspaceConfiguration } from 'vscode';
import {
GlobalEnvironmentVariableCollection,
Uri,
WorkspaceConfiguration,
Disposable,
CancellationToken,
TerminalLinkContext,
Terminal,
EventEmitter,
} from 'vscode';
import { assert } from 'chai';
import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis';
import { registerPythonStartup } from '../../../client/terminals/pythonStartup';
import { IExtensionContext } from '../../../client/common/types';
import * as pythonStartupLinkProvider from '../../../client/terminals/pythonStartupLinkProvider';
import { CustomTerminalLinkProvider } from '../../../client/terminals/pythonStartupLinkProvider';
import { Repl } from '../../../client/common/utils/localize';

suite('Terminal - Shell Integration with PYTHONSTARTUP', () => {
let getConfigurationStub: sinon.SinonStub;
Expand All @@ -20,7 +33,6 @@ suite('Terminal - Shell Integration with PYTHONSTARTUP', () => {
setup(() => {
context = TypeMoq.Mock.ofType<IExtensionContext>();
globalEnvironmentVariableCollection = TypeMoq.Mock.ofType<GlobalEnvironmentVariableCollection>();

// Question: Why do we have to set up environmentVariableCollection and globalEnvironmentVariableCollection in this flip-flop way?
// Reference: /vscode-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts
context.setup((c) => c.environmentVariableCollection).returns(() => globalEnvironmentVariableCollection.object);
Expand Down Expand Up @@ -122,4 +134,64 @@ suite('Terminal - Shell Integration with PYTHONSTARTUP', () => {

globalEnvironmentVariableCollection.verify((c) => c.delete('PYTHONSTARTUP'), TypeMoq.Times.once());
});

test('Ensure registering terminal link calls registerTerminalLinkProvider', async () => {
anthonykim1 marked this conversation as resolved.
Show resolved Hide resolved
const registerTerminalLinkProviderStub = sinon.stub(
pythonStartupLinkProvider,
'registerCustomTerminalLinkProvider',
);
const disposableArray: Disposable[] = [];
pythonStartupLinkProvider.registerCustomTerminalLinkProvider(disposableArray);

sinon.assert.calledOnce(registerTerminalLinkProviderStub);
sinon.assert.calledWith(registerTerminalLinkProviderStub, disposableArray);

registerTerminalLinkProviderStub.restore();
});

test('Verify provideTerminalLinks returns links when context.line contains expectedNativeLink', () => {
const provider = new CustomTerminalLinkProvider();
const context: TerminalLinkContext = {
line: 'Some random string with VS Code Native REPL in it',
terminal: {} as Terminal,
};
const token: CancellationToken = {
isCancellationRequested: false,
onCancellationRequested: new EventEmitter<unknown>().event,
};

const links = provider.provideTerminalLinks(context, token);

assert.isNotNull(links, 'Expected links to be not undefined');
assert.isArray(links, 'Expected links to be an array');
Comment on lines +165 to +166
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check link.command here and validate start and end lengths. if you put the localized string in localize.ts then you can verify tooltip.

You also need a negative test case.

assert.isNotEmpty(links, 'Expected links to be not empty');

if (Array.isArray(links)) {
assert.equal(links[0].command, 'python.startNativeREPL', 'Expected command to be python.startNativeREPL');
assert.equal(
links[0].startIndex,
context.line.indexOf('VS Code Native REPL'),
'Expected startIndex to be 0',
);
assert.equal(links[0].length, 'VS Code Native REPL'.length, 'Expected length to be 16');
assert.equal(links[0].tooltip, Repl.launchNativeRepl, 'Expected tooltip to be Launch VS Code Native REPL');
}
});

test('Verify provideTerminalLinks returns no links when context.line does not contain expectedNativeLink', () => {
const provider = new CustomTerminalLinkProvider();
const context: TerminalLinkContext = {
line: 'Some random string without the expected link',
terminal: {} as Terminal,
};
const token: CancellationToken = {
isCancellationRequested: false,
onCancellationRequested: new EventEmitter<unknown>().event,
};

const links = provider.provideTerminalLinks(context, token);

assert.isArray(links, 'Expected links to be an array');
assert.isEmpty(links, 'Expected links to be empty');
});
});
Loading