Skip to content

Commit

Permalink
Show warning and allow user to turn off smart send for deprecated Pyt…
Browse files Browse the repository at this point in the history
…hon code (microsoft#22353)

Resolves: microsoft#22341 microsoft#22340

Showing warning message after detecting user is on Python file with
deprecated Python code, and are attempting to run smart send via
shift+enter action. Allow user to turn off this via workspace setting.

---------

Co-authored-by: Courtney Webster <60238438+cwebster-99@users.noreply.github.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
  • Loading branch information
3 people authored Nov 18, 2023
1 parent ef983f4 commit 3c552f9
Show file tree
Hide file tree
Showing 14 changed files with 184 additions and 17 deletions.
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,12 @@
"scope": "resource",
"type": "array"
},
"python.REPL.enableREPLSmartSend": {
"default": true,
"description": "%python.EnableREPLSmartSend.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 @@ -59,6 +59,7 @@
"python.missingPackage.severity.description": "Set severity of missing packages in requirements.txt or pyproject.toml",
"python.pipenvPath.description": "Path to the pipenv executable to use for activation.",
"python.poetryPath.description": "Path to the poetry 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.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
27 changes: 22 additions & 5 deletions pythonFiles/normalizeSelection.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,18 @@ def traverse_file(wholeFileContent, start_line, end_line, was_highlighted):
or a multiline dictionary, or differently styled multi-line list comprehension, etc.
Then call the normalize_lines function to normalize our smartly selected code block.
"""
parsed_file_content = None

try:
parsed_file_content = ast.parse(wholeFileContent)
except Exception:
# Handle case where user is attempting to run code where file contains deprecated Python code.
# Let typescript side know and show warning message.
return {
"normalized_smart_result": "deprecated",
"which_line_next": 0,
}

parsed_file_content = ast.parse(wholeFileContent)
smart_code = ""
should_run_top_blocks = []

Expand Down Expand Up @@ -267,7 +277,11 @@ def get_next_block_lineno(which_line_next):
data = None
which_line_next = 0

if empty_Highlight and contents.get("smartSendExperimentEnabled"):
if (
empty_Highlight
and contents.get("smartSendExperimentEnabled")
and contents.get("smartSendSettingsEnabled")
):
result = traverse_file(
contents["wholeFileContent"],
vscode_start_line,
Expand All @@ -276,9 +290,12 @@ def get_next_block_lineno(which_line_next):
)
normalized = result["normalized_smart_result"]
which_line_next = result["which_line_next"]
data = json.dumps(
{"normalized": normalized, "nextBlockLineno": result["which_line_next"]}
)
if normalized == "deprecated":
data = json.dumps({"normalized": normalized})
else:
data = json.dumps(
{"normalized": normalized, "nextBlockLineno": result["which_line_next"]}
)
else:
normalized = normalize_lines(contents["code"])
data = json.dumps({"normalized": normalized})
Expand Down
1 change: 1 addition & 0 deletions src/client/common/application/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
];
['workbench.action.files.openFolder']: [];
['workbench.action.openWorkspace']: [];
['workbench.action.openSettings']: [string];
['setContext']: [string, boolean] | ['python.vscode.channel', Channel];
['python.reloadVSCode']: [string];
['revealLine']: [{ lineNumber: number; at: 'top' | 'center' | 'bottom' }];
Expand Down
6 changes: 5 additions & 1 deletion src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
IInterpreterPathService,
IInterpreterSettings,
IPythonSettings,
IREPLSettings,
ITensorBoardSettings,
ITerminalSettings,
Resource,
Expand Down Expand Up @@ -111,6 +112,8 @@ export class PythonSettings implements IPythonSettings {

public globalModuleInstallation = false;

public REPL!: IREPLSettings;

public experiments!: IExperiments;

public languageServer: LanguageServerType = LanguageServerType.Node;
Expand Down Expand Up @@ -360,7 +363,8 @@ export class PythonSettings implements IPythonSettings {
activateEnvInCurrentTerminal: false,
};

const experiments = systemVariables.resolveAny(pythonSettings.get<IExperiments>('experiments'))!;
this.REPL = pythonSettings.get<IREPLSettings>('REPL')!;
const experiments = pythonSettings.get<IExperiments>('experiments')!;
if (this.experiments) {
Object.assign<IExperiments, IExperiments>(this.experiments, experiments);
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/client/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export interface IPythonSettings {
readonly languageServerIsDefault: boolean;
readonly defaultInterpreterPath: string;
readonly tensorBoard: ITensorBoardSettings | undefined;
readonly REPL: IREPLSettings;
register(): void;
}

Expand All @@ -197,6 +198,10 @@ export interface ITerminalSettings {
readonly activateEnvInCurrentTerminal: boolean;
}

export interface IREPLSettings {
readonly enableREPLSmartSend: boolean;
}

export interface IExperiments {
/**
* Return `true` if experiments are enabled, else `false`.
Expand Down
7 changes: 7 additions & 0 deletions src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
'use strict';

import { l10n } from 'vscode';
import { Commands } from '../constants';

/* eslint-disable @typescript-eslint/no-namespace, no-shadow */

Expand Down Expand Up @@ -42,6 +43,9 @@ export namespace Diagnostics {
export const pylanceDefaultMessage = l10n.t(
"The Python extension now includes Pylance to improve completions, code navigation, overall performance and much more! You can learn more about the update and learn how to change your language server [here](https://aka.ms/new-python-bundle).\n\nRead Pylance's license [here](https://marketplace.visualstudio.com/items/ms-python.vscode-pylance/license).",
);
export const invalidSmartSendMessage = l10n.t(`Python is unable to parse the code provided. Please
turn off Smart Send if you wish to always run line by line or explicitly select code
to force run. See [logs](command:${Commands.ViewOutput}) for more details`);
}

export namespace Common {
Expand Down Expand Up @@ -92,6 +96,9 @@ export namespace AttachProcess {
export const refreshList = l10n.t('Refresh process list');
}

export namespace Repl {
export const disableSmartSend = l10n.t('Disable Smart Send');
}
export namespace Pylance {
export const remindMeLater = l10n.t('Remind me later');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
disposableRegistry,
platformService,
interpreterService,
commandManager,
);
this.terminalTitle = 'Django Shell';
disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));
Expand Down
19 changes: 18 additions & 1 deletion src/client/terminals/codeExecution/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify';
import { l10n, Position, Range, TextEditor, Uri } from 'vscode';

import {
IActiveResourceService,
IApplicationShell,
ICommandManager,
IDocumentManager,
Expand Down Expand Up @@ -36,6 +37,8 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {

private readonly commandManager: ICommandManager;

private activeResourceService: IActiveResourceService;

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error TS6133: 'configSettings' is declared but its value is never read.
private readonly configSettings: IConfigurationService;
Expand All @@ -47,6 +50,7 @@ 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);
}

public async normalizeLines(code: string, wholeFileContent?: string, resource?: Uri): Promise<string> {
Expand Down Expand Up @@ -90,13 +94,21 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
const endLineVal = activeEditor?.selection?.end.line ?? 0;
const emptyHighlightVal = activeEditor?.selection?.isEmpty ?? true;
const smartSendExperimentEnabledVal = pythonSmartSendEnabled(this.serviceContainer);
let smartSendSettingsEnabledVal = false;
const configuration = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
if (configuration) {
const pythonSettings = configuration.getSettings(this.activeResourceService.getActiveResource());
smartSendSettingsEnabledVal = pythonSettings.REPL.enableREPLSmartSend;
}

const input = JSON.stringify({
code,
wholeFileContent,
startLine: startLineVal,
endLine: endLineVal,
emptyHighlight: emptyHighlightVal,
smartSendExperimentEnabled: smartSendExperimentEnabledVal,
smartSendSettingsEnabled: smartSendSettingsEnabledVal,
});
observable.proc?.stdin?.write(input);
observable.proc?.stdin?.end();
Expand All @@ -105,7 +117,12 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
const result = await normalizeOutput.promise;
const object = JSON.parse(result);

if (activeEditor?.selection) {
if (
activeEditor?.selection &&
smartSendExperimentEnabledVal &&
smartSendSettingsEnabledVal &&
object.normalized !== 'deprecated'
) {
const lineOffset = object.nextBlockLineno - activeEditor!.selection.start.line - 1;
await this.moveToNextBlock(lineOffset, activeEditor);
}
Expand Down
4 changes: 3 additions & 1 deletion src/client/terminals/codeExecution/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { inject, injectable } from 'inversify';
import { Disposable } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { ICommandManager, IWorkspaceService } from '../../common/application/types';
import { IPlatformService } from '../../common/platform/types';
import { ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
Expand All @@ -21,6 +21,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
@inject(IPlatformService) platformService: IPlatformService,
@inject(IInterpreterService) interpreterService: IInterpreterService,
@inject(ICommandManager) commandManager: ICommandManager,
) {
super(
terminalServiceFactory,
Expand All @@ -29,6 +30,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
disposableRegistry,
platformService,
interpreterService,
commandManager,
);
this.terminalTitle = 'REPL';
}
Expand Down
19 changes: 15 additions & 4 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
import { inject, injectable } from 'inversify';
import * as path from 'path';
import { Disposable, Uri } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { ICommandManager, IWorkspaceService } from '../../common/application/types';
import '../../common/extensions';
import { IPlatformService } from '../../common/platform/types';
import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
import { Diagnostics, Repl } from '../../common/utils/localize';
import { showWarningMessage } from '../../common/vscodeApis/windowApis';
import { IInterpreterService } from '../../interpreter/contracts';
import { traceInfo } from '../../logging';
import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
import { ICodeExecutionService } from '../../terminals/types';

@injectable()
export class TerminalCodeExecutionProvider implements ICodeExecutionService {
private hasRanOutsideCurrentDrive = false;
Expand All @@ -27,6 +29,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
@inject(IDisposableRegistry) protected readonly disposables: Disposable[],
@inject(IPlatformService) protected readonly platformService: IPlatformService,
@inject(IInterpreterService) protected readonly interpreterService: IInterpreterService,
@inject(ICommandManager) protected readonly commandManager: ICommandManager,
) {}

public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) {
Expand All @@ -42,9 +45,17 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
if (!code || code.trim().length === 0) {
return;
}

await this.initializeRepl(resource);
await this.getTerminalService(resource).sendText(code);
if (code == 'deprecated') {
// If user is trying to smart send deprecated code show warning
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);
}
} else {
await this.getTerminalService(resource).sendText(code);
}
}
public async initializeRepl(resource: Resource) {
const terminalService = this.getTerminalService(resource);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
def beliebig(x, y, *mehr):
print "x=", x, ", x=", y
print "mehr: ", mehr

list = [
1,
2,
3,
]
print("Above is invalid");print("deprecated");print("show warning")
Loading

0 comments on commit 3c552f9

Please sign in to comment.