Skip to content

Commit

Permalink
settings: Remove settings related to upstream python REPL (#5931)
Browse files Browse the repository at this point in the history
<!-- Thank you for submitting a pull request.
If this is your first pull request you can find information about
contributing here:
  * https://github.com/posit-dev/positron/blob/main/CONTRIBUTING.md

We recommend synchronizing your branch with the latest changes in the
main branch by either pulling or rebasing.
-->

<!--
  Describe briefly what problem this pull request resolves, or what
  new feature it introduces. Include screenshots of any new or altered
  UI. Link to any GitHub issues but avoid "magic" keywords that will
  automatically close the issue. If there are any details about your
  approach that are unintuitive or you want to draw attention to, please
  describe them here.
-->
This PR removes the `python.REPL.*` settings that come from the
upstream, which aren't useful in Positron since we don't use the python
REPL.


### Release Notes

<!--
Optionally, replace `N/A` with text to be included in the next release
notes.
The `N/A` bullets are ignored. If you refer to one or more Positron
issues,
these issues are used to collect information about the feature or
bugfix, such
as the relevant language pack as determined by Github labels of type
`lang: `.
  The note will automatically be tagged with the language.

These notes are typically filled by the Positron team. If you are an
external
  contributor, you may ignore this section.
-->

#### New Features

- N/A

#### Bug Fixes

- Removed some redundant settings related to the upstream python REPL
(#5602)


### QA Notes

<!--
  Add additional information for QA on how to validate the change,
  paying special attention to the level of risk, adjacent areas that
  could be affected by the change, and any important contextual
  information not present in the linked issues.
-->
I validated that these settings don't appear in the settings menu
anymore.
  • Loading branch information
austin3dickey authored Jan 15, 2025
1 parent fe3a53c commit 271dbb5
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 96 deletions.
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 ---

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

0 comments on commit 271dbb5

Please sign in to comment.