Skip to content

Commit

Permalink
enable turning off the variable provider (#24231)
Browse files Browse the repository at this point in the history
along with microsoft/vscode#230349

let people disable the feature in case it causes perf issues
  • Loading branch information
amunger authored Oct 3, 2024
1 parent c60f0dd commit 3362dc3
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 3 deletions.
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,12 @@
"experimental"
]
},
"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
1 change: 1 addition & 0 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
"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
17 changes: 15 additions & 2 deletions src/client/repl/variables/variablesProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import {
EventEmitter,
Event,
NotebookVariableProvider,
Uri,
} from 'vscode';
import { VariableResultCache } from './variableResultCache';
import { IVariableDescription } from './types';
import { VariableRequester } from './variableRequester';
import { getConfiguration } from '../../common/vscodeApis/workspaceApis';

export class VariablesProvider implements NotebookVariableProvider {
private readonly variableResultCache = new VariableResultCache();
Expand All @@ -36,7 +38,9 @@ export class VariablesProvider implements NotebookVariableProvider {
const notebook = this.getNotebookDocument();
if (notebook) {
this.executionCount += 1;
this._onDidChangeVariables.fire(notebook);
if (isEnabled(notebook.uri)) {
this._onDidChangeVariables.fire(notebook);
}
}
}

Expand All @@ -48,7 +52,12 @@ export class VariablesProvider implements NotebookVariableProvider {
token: CancellationToken,
): AsyncIterable<VariablesResult> {
const notebookDocument = this.getNotebookDocument();
if (token.isCancellationRequested || !notebookDocument || notebookDocument !== notebook) {
if (
!isEnabled(notebook.uri) ||
token.isCancellationRequested ||
!notebookDocument ||
notebookDocument !== notebook
) {
return;
}

Expand Down Expand Up @@ -144,3 +153,7 @@ function getVariableResultCacheKey(uri: string, parent: Variable | undefined, st
}
return `${uri}:${parentKey}`;
}

function isEnabled(resource?: Uri) {
return getConfiguration('python', resource).get('REPL.provideVariables');
}
61 changes: 60 additions & 1 deletion src/test/repl/variableProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,29 @@
// Licensed under the MIT License.

import { assert } from 'chai';
import { NotebookDocument, CancellationTokenSource, VariablesResult, Variable, EventEmitter } from 'vscode';
import sinon from 'sinon';
import {
NotebookDocument,
CancellationTokenSource,
VariablesResult,
Variable,
EventEmitter,
ConfigurationScope,
WorkspaceConfiguration,
} from 'vscode';
import * as TypeMoq from 'typemoq';
import { IVariableDescription } from '../../client/repl/variables/types';
import { VariablesProvider } from '../../client/repl/variables/variablesProvider';
import { VariableRequester } from '../../client/repl/variables/variableRequester';
import * as workspaceApis from '../../client/common/vscodeApis/workspaceApis';

suite('ReplVariablesProvider', () => {
let provider: VariablesProvider;
let varRequester: TypeMoq.IMock<VariableRequester>;
let notebook: TypeMoq.IMock<NotebookDocument>;
let getConfigurationStub: sinon.SinonStub;
let configMock: TypeMoq.IMock<WorkspaceConfiguration>;
let enabled: boolean;
const executionEventEmitter = new EventEmitter<void>();
const cancellationToken = new CancellationTokenSource().token;

Expand Down Expand Up @@ -68,9 +81,23 @@ suite('ReplVariablesProvider', () => {
}

setup(() => {
enabled = true;
varRequester = TypeMoq.Mock.ofType<VariableRequester>();
notebook = TypeMoq.Mock.ofType<NotebookDocument>();
provider = new VariablesProvider(varRequester.object, () => notebook.object, executionEventEmitter.event);
configMock = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
configMock.setup((c) => c.get<boolean>('REPL.provideVariables')).returns(() => enabled);
getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
getConfigurationStub.callsFake((section?: string, _scope?: ConfigurationScope | null) => {
if (section === 'python') {
return configMock.object;
}
return undefined;
});
});

teardown(() => {
sinon.restore();
});

test('provideVariables without parent should yield variables', async () => {
Expand All @@ -84,6 +111,38 @@ 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]);

const results = await provideVariables(undefined);

assert.isEmpty(results);
});

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

executionEventEmitter.fire();

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

test('Variables change event from provider should fire when execution happens', async () => {
let eventFired = false;
provider.onDidChangeVariables(() => {
eventFired = true;
});

executionEventEmitter.fire();

assert.isTrue(eventFired, 'event should have fired');
});

test('provideVariables with a parent should call get children correctly', async () => {
const listVariableItems = [0, 1, 2].map(createListItem);
setVariablesForParent(undefined, [objectVariable]);
Expand Down

0 comments on commit 3362dc3

Please sign in to comment.