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

settings: Remove settings related to upstream python REPL #5931

Merged
merged 6 commits into from
Jan 15, 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
22 changes: 0 additions & 22 deletions extensions/positron-python/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -773,28 +773,6 @@
"preview"
]
},
"python.REPL.enableREPLSmartSend": {
"default": true,
"description": "%python.EnableREPLSmartSend.description%",
"scope": "resource",
"type": "boolean"
},
"python.REPL.sendToNativeREPL": {
"default": false,
"description": "%python.REPL.sendToNativeREPL.description%",
"scope": "resource",
"type": "boolean",
"tags": [
"onExP",
"preview"
]
},
"python.REPL.provideVariables": {
"default": true,
"description": "%python.REPL.provideVariables.description%",
"scope": "resource",
"type": "boolean"
},
"python.testing.autoTestDiscoverOnSaveEnabled": {
"default": true,
"description": "%python.testing.autoTestDiscoverOnSaveEnabled.description%",
Expand Down
3 changes: 0 additions & 3 deletions extensions/positron-python/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@
"python.poetryPath.description": "Path to the poetry executable.",
"python.quietMode.description": "Start Positron's IPython shell in quiet mode, to suppress initial version and help messages (restart Positron to apply).",
"python.pixiToolPath.description": "Path to the pixi executable.",
"python.EnableREPLSmartSend.description": "Toggle Smart Send for the Python REPL. Smart Send enables sending the smallest runnable block of code to the REPL on Shift+Enter and moves the cursor accordingly.",
"python.REPL.sendToNativeREPL.description": "Toggle to send code to Python REPL instead of the terminal on execution. Turning this on will change the behavior for both Smart Send and Run Selection/Line in the Context Menu.",
"python.REPL.provideVariables.description": "Toggle to provide variables for the REPL variable view for the native REPL.",
"python.tensorBoard.logDirectory.description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.",
"python.tensorBoard.logDirectory.markdownDeprecationMessage": "Tensorboard support has been moved to the extension [Tensorboard extension](https://marketplace.visualstudio.com/items?itemName=ms-toolsai.tensorboard). Instead use the setting `tensorBoard.logDirectory`.",
"python.tensorBoard.logDirectory.deprecationMessage": "Tensorboard support has been moved to the extension Tensorboard extension. Instead use the setting `tensorBoard.logDirectory`.",
Expand Down
5 changes: 5 additions & 0 deletions extensions/positron-python/src/client/repl/replUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ export async function getSelectedTextToExecute(textEditor: TextEditor): Promise<
* @returns boolean - True if sendToNativeREPL setting is enabled, False otherwise.
*/
export function getSendToNativeREPLSetting(): boolean {
// --- Start Positron ---
// This setting is hidden in favor of the Positron console.
// We leave the dead code path below to make merge conflicts easier to resolve.
return false;
// --- End Positron ---
const uri = getActiveResource();
const configuration = getConfiguration('python', uri);
return configuration.get<boolean>('REPL.sendToNativeREPL', false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,10 @@ function getVariableResultCacheKey(uri: string, parent: Variable | undefined, st
}

function isEnabled(resource?: Uri) {
// --- Start Positron ---
// This setting is hidden in favor of the Positron console.
// We leave the dead code path below to make merge conflicts easier to resolve.
return true;
// --- End Positron ---
return getConfiguration('python', resource).get('REPL.provideVariables');
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { inject, injectable } from 'inversify';
import { l10n, Position, Range, TextEditor, Uri } from 'vscode';

import {
IActiveResourceService,
// --- Start Positron ---
// IActiveResourceService,
// --- End Positron ---
IApplicationShell,
ICommandManager,
IDocumentManager,
Expand Down Expand Up @@ -36,7 +38,9 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {

private readonly commandManager: ICommandManager;

private activeResourceService: IActiveResourceService;
// --- Start Positron ---
// private activeResourceService: IActiveResourceService;
// --- End Positron ---

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error TS6133: 'configSettings' is declared but its value is never read.
Expand All @@ -49,7 +53,9 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
this.interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
this.configSettings = serviceContainer.get<IConfigurationService>(IConfigurationService);
this.commandManager = serviceContainer.get<ICommandManager>(ICommandManager);
this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
// --- Start Positron ---
// this.activeResourceService = this.serviceContainer.get<IActiveResourceService>(IActiveResourceService);
// --- End Positron ---
}

public async normalizeLines(code: string, wholeFileContent?: string, resource?: Uri): Promise<string> {
Expand Down Expand Up @@ -92,12 +98,16 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
const startLineVal = activeEditor?.selection?.start.line ?? 0;
const endLineVal = activeEditor?.selection?.end.line ?? 0;
const emptyHighlightVal = activeEditor?.selection?.isEmpty ?? true;
let smartSendSettingsEnabledVal = true;
const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
if (configuration) {
const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource());
smartSendSettingsEnabledVal = pythonSettings.REPL.enableREPLSmartSend;
}
// --- Start Positron ---
// This setting is hidden in favor of the Positron console.
const smartSendSettingsEnabledVal = true;
// let smartSendSettingsEnabledVal = true;
// const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
// if (configuration) {
// const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource());
// smartSendSettingsEnabledVal = pythonSettings.REPL.enableREPLSmartSend;
// }
// --- End Positron ---
austin3dickey marked this conversation as resolved.
Show resolved Hide resolved

const input = JSON.stringify({
code,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
const selection = await showWarningMessage(Diagnostics.invalidSmartSendMessage, Repl.disableSmartSend);
traceInfo(`Selected file contains invalid Python or Deprecated Python 2 code`);
if (selection === Repl.disableSmartSend) {
this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource);
// --- Start Positron ---
// This setting is hidden in favor of the Positron console.
// this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource);
// --- End Positron ---
}
} else {
await this.getTerminalService(resource).executeCommand(code);
Expand Down
34 changes: 19 additions & 15 deletions extensions/positron-python/src/test/repl/variableProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,26 +111,30 @@ suite('ReplVariablesProvider', () => {
assert.equal(results[0].variable.expression, 'myObject');
});

test('No variables are returned when variable provider is disabled', async () => {
enabled = false;
setVariablesForParent(undefined, [objectVariable]);
// --- Start Positron ---
// The setting for disabling the variable provider is hidden, so it's always enabled.

const results = await provideVariables(undefined);
// test('No variables are returned when variable provider is disabled', async () => {
// enabled = false;
// setVariablesForParent(undefined, [objectVariable]);

assert.isEmpty(results);
});
// const results = await provideVariables(undefined);

test('No change event from provider when disabled', async () => {
enabled = false;
let eventFired = false;
provider.onDidChangeVariables(() => {
eventFired = true;
});
// assert.isEmpty(results);
// });

executionEventEmitter.fire();
// test('No change event from provider when disabled', async () => {
// enabled = false;
// let eventFired = false;
// provider.onDidChangeVariables(() => {
// eventFired = true;
// });

assert.isFalse(eventFired, 'event should not have fired');
});
// executionEventEmitter.fire();

// assert.isFalse(eventFired, 'event should not have fired');
// });
// --- End Positron ---

test('Variables change event from provider should fire when execution happens', async () => {
let eventFired = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,30 @@
'use strict';

import { expect } from 'chai';
import * as path from 'path';
// --- Start Positron ---
// import * as path from 'path';
// --- End Positron ---
import { SemVer } from 'semver';
import * as TypeMoq from 'typemoq';
import { Position, Range, Selection, TextDocument, TextEditor, TextLine, Uri } from 'vscode';
import * as fs from '../../../client/common/platform/fs-paths';
// --- Start Positron ---
// import * as fs from '../../../client/common/platform/fs-paths';
// --- End Positron ---
import {
IActiveResourceService,
IApplicationShell,
ICommandManager,
IDocumentManager,
IWorkspaceService,
} from '../../../client/common/application/types';
import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../../client/common/constants';
// --- Start Positron ---
// import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../../client/common/constants';
import { PYTHON_LANGUAGE } from '../../../client/common/constants';
// --- End Positron ---
import '../../../client/common/extensions';
import { ProcessService } from '../../../client/common/process/proc';
// --- Start Positron ---
// import { ProcessService } from '../../../client/common/process/proc';
// --- End Positron ---
import {
IProcessService,
IProcessServiceFactory,
Expand All @@ -34,7 +43,9 @@ import { CodeExecutionHelper } from '../../../client/terminals/codeExecution/hel
import { ICodeExecutionHelper } from '../../../client/terminals/types';
import { PYTHON_PATH } from '../../common';

const TEST_FILES_PATH = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'python_files', 'terminalExec');
// --- Start Positron ---
// const TEST_FILES_PATH = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'python_files', 'terminalExec');
// --- End Positron ---

suite('Terminal - Code Execution Helper', () => {
let activeResourceService: TypeMoq.IMock<IActiveResourceService>;
Expand Down Expand Up @@ -149,47 +160,54 @@ suite('Terminal - Code Execution Helper', () => {
expect(execArgs).to.contain('normalizeSelection.py');
});

async function ensureCodeIsNormalized(source: string, expectedSource: string) {
configurationService
.setup((c) => c.getSettings(TypeMoq.It.isAny()))
.returns({
REPL: {
EnableREPLSmartSend: false,
REPLSmartSend: false,
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
const actualProcessService = new ProcessService();
processService
.setup((p) => p.execObservable(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
.returns((file, args, options) =>
actualProcessService.execObservable.apply(actualProcessService, [file, args, options]),
);
const normalizedCode = await helper.normalizeLines(source);
const normalizedExpected = expectedSource.replace(/\r\n/g, '\n');
expect(normalizedCode).to.be.equal(normalizedExpected);
}

['', '1', '2', '3', '4', '5', '6', '7', '8'].forEach((fileNameSuffix) => {
test(`Ensure code is normalized (Sample${fileNameSuffix})`, async () => {
configurationService
.setup((c) => c.getSettings(TypeMoq.It.isAny()))
.returns({
REPL: {
EnableREPLSmartSend: false,
REPLSmartSend: false,
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
const code = await fs.readFile(path.join(TEST_FILES_PATH, `sample${fileNameSuffix}_raw.py`), 'utf8');
const expectedCode = await fs.readFile(
path.join(TEST_FILES_PATH, `sample${fileNameSuffix}_normalized_selection.py`),
'utf8',
);

await ensureCodeIsNormalized(code, expectedCode);
});
});
// --- Start Positron ---
// These tests are disabled because they require the EnableREPLSmartSend setting to be disabled.
// In Positron that setting is hidden (see https://github.com/posit-dev/positron/pull/5931) so
// it's always enabled.

// async function ensureCodeIsNormalized(source: string, expectedSource: string) {
// configurationService
// .setup((c) => c.getSettings(TypeMoq.It.isAny()))
// .returns({
// REPL: {
// EnableREPLSmartSend: false,
// REPLSmartSend: false,
// },
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
// } as any);
// const actualProcessService = new ProcessService();
// processService
// .setup((p) => p.execObservable(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
// .returns((file, args, options) =>
// actualProcessService.execObservable.apply(actualProcessService, [file, args, options]),
// );
// const normalizedCode = await helper.normalizeLines(source);
// const normalizedExpected = expectedSource.replace(/\r\n/g, '\n');
// expect(normalizedCode).to.be.equal(normalizedExpected);
// }

// ['', '1', '2', '3', '4', '5', '6', '7', '8'].forEach((fileNameSuffix) => {
// test(`Ensure code is normalized (Sample${fileNameSuffix})`, async () => {
// configurationService
// .setup((c) => c.getSettings(TypeMoq.It.isAny()))
// .returns({
// REPL: {
// EnableREPLSmartSend: false,
// REPLSmartSend: false,
// },
// // eslint-disable-next-line @typescript-eslint/no-explicit-any
// } as any);
// const code = await fs.readFile(path.join(TEST_FILES_PATH, `sample${fileNameSuffix}_raw.py`), 'utf8');
// const expectedCode = await fs.readFile(
// path.join(TEST_FILES_PATH, `sample${fileNameSuffix}_normalized_selection.py`),
// 'utf8',
// );

// await ensureCodeIsNormalized(code, expectedCode);
// });
// });

// --- End Positron ---

test("Display message if there's no active file", async () => {
documentManager.setup((doc) => doc.activeTextEditor).returns(() => undefined);
Expand Down
Loading