Skip to content

Commit

Permalink
Simpler normalization for executing selection in IW (#14449)
Browse files Browse the repository at this point in the history
* option to not normalize selected code

* unit tests

* working unit tests

* register setting

* lint

* enable IW commands in web

* test multi-line string

* remove suite only
  • Loading branch information
amunger authored Oct 9, 2023
1 parent ecfc867 commit 450e4d0
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 18 deletions.
12 changes: 9 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@
"command": "jupyter.execSelectionInteractive",
"title": "%jupyter.command.jupyter.execSelectionInteractive.title%",
"category": "Jupyter",
"enablement": "isWorkspaceTrusted && !jupyter.webExtension"
"enablement": "isWorkspaceTrusted"
},
{
"command": "jupyter.createnewinteractive",
Expand Down Expand Up @@ -545,13 +545,13 @@
"command": "jupyter.runtoline",
"title": "%jupyter.command.jupyter.runtoline.title%",
"category": "Jupyter",
"enablement": "isWorkspaceTrusted && !jupyter.webExtension"
"enablement": "isWorkspaceTrusted"
},
{
"command": "jupyter.runfromline",
"title": "%jupyter.command.jupyter.runfromline.title%",
"category": "Jupyter",
"enablement": "isWorkspaceTrusted && !jupyter.webExtension"
"enablement": "isWorkspaceTrusted"
},
{
"command": "jupyter.importnotebook",
Expand Down Expand Up @@ -1516,6 +1516,12 @@
"description": "%jupyter.configuration.jupyter.sendSelectionToInteractiveWindow.description%",
"scope": "resource"
},
"jupyter.interactiveWindow.textEditor.normalizeSelection": {
"type": "boolean",
"default": true,
"description": "%jupyter.configuration.jupyter.normalizeSelectionForInteractiveWindow.description%",
"scope": "resource"
},
"jupyter.variableExplorerExclude": {
"type": "string",
"default": "module;function;builtin_function_or_method;ABCMeta;type;ModelMetaclass",
Expand Down
3 changes: 2 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@
"jupyter.configuration.jupyter.useDefaultConfigForJupyter.description": "When running Jupyter locally, create a default empty Jupyter config",
"jupyter.configuration.jupyter.jupyterInterruptTimeout.description": "Amount of time (in ms) to wait for an interrupt before asking to restart the Jupyter kernel.",
"jupyter.configuration.jupyter.sendSelectionToInteractiveWindow.description": "When pressing shift+enter, send selected code in a Python file to the Jupyter interactive window as opposed to the Python terminal.",
"jupyter.configuration.jupyter.normalizeSelectionForInteractiveWindow.description": "Selected text will be normalized before it is executed in the Interactive Window.",
"jupyter.configuration.jupyter.sendSelectionToInteractiveWindow.deprecationMessage": "This setting has been deprecated in favor of jupyter.interactiveWindow.textEditor.executeSelection.",
"jupyter.configuration.jupyter.variableExplorerExclude.description": "Types to exclude from showing in the Interactive variable explorer",
"jupyter.configuration.jupyter.codeRegularExpression.description": "Regular expression used to identify code cells. All code until the next match is considered part of this cell.",
Expand Down Expand Up @@ -292,4 +293,4 @@
"DataScience.installPythonTitle": "Install Python",
"DataScience.installPythonExtensionViaKernelPickerTitle": "Install Python Extension",
"DataScience.switchToRemoteKernelsTitle": "Connect to a Jupyter Server"
}
}
8 changes: 7 additions & 1 deletion src/interactive-window/editor-integration/codewatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,13 @@ export class CodeWatcher implements ICodeWatcher {
if (!codeToExecute) {
return;
}
const normalizedCode = await this.executionHelper.normalizeLines(codeToExecute!);

const normalize =
this.configService.getSettings(activeEditor.document.uri).normalizeSelectionForInteractiveWindow ??
true;
const normalizedCode = normalize
? await this.executionHelper.normalizeLines(codeToExecute!)
: codeToExecute;
if (!normalizedCode || normalizedCode.trim().length === 0) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import {
} from '../../platform/common/application/types';
import { IFileSystem } from '../../platform/common/platform/types';
import { IConfigurationService } from '../../platform/common/types';
import { CodeLensFactory } from '../../interactive-window/editor-integration/codeLensFactory';
import { DataScienceCodeLensProvider } from '../../interactive-window/editor-integration/codelensprovider';
import { CodeWatcher } from '../../interactive-window/editor-integration/codewatcher';
import { CodeLensFactory } from './codeLensFactory';
import { DataScienceCodeLensProvider } from './codelensprovider';
import { CodeWatcher } from './codewatcher';
import { IServiceContainer } from '../../platform/ioc/types';
import { ICodeExecutionHelper } from '../../platform/terminals/types';
import { dispose } from '../../platform/common/helpers';
import { IKernel, IKernelProvider } from '../../kernels/types';
import { InteractiveCellResultError } from '../../platform/errors/interactiveCellResultError';
import { ICodeWatcher, IGeneratedCodeStorageFactory } from '../../interactive-window/editor-integration/types';
import { ICodeWatcher, IGeneratedCodeStorageFactory } from './types';
import { IInteractiveWindowProvider, IInteractiveWindow } from '../../interactive-window/types';
import { Commands, EditorContexts } from '../../platform/common/constants';
import { SystemVariables } from '../../platform/common/variables/systemVariables.node';
Expand Down
1 change: 1 addition & 0 deletions src/platform/common/configMigration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export class ConfigMigration {
interactiveWindowViewColumn: 'interactiveWindow.viewColumn',

sendSelectionToInteractiveWindow: 'interactiveWindow.textEditor.executeSelection',
normalizeSelectionForInteractiveWindow: 'interactiveWindow.textEditor.normalizeSelection',
magicCommandsAsComments: 'interactiveWindow.textEditor.magicCommandsAsComments',
enableAutoMoveToNextCell: 'interactiveWindow.textEditor.autoMoveToNextCell',
newCellOnRunLast: 'interactiveWindow.textEditor.autoAddNewCell',
Expand Down
1 change: 1 addition & 0 deletions src/platform/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class JupyterSettings implements IWatchableJupyterSettings {
public useDefaultConfigForJupyter: boolean = false;
public searchForJupyter: boolean = false;
public sendSelectionToInteractiveWindow: boolean = false;
public normalizeSelectionForInteractiveWindow: boolean = true;
public markdownRegularExpression: string = '';
public codeRegularExpression: string = '';
public errorBackgroundColor: string = '';
Expand Down
1 change: 1 addition & 0 deletions src/platform/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export interface IJupyterSettings {
readonly searchForJupyter: boolean;
readonly enablePythonKernelLogging: boolean;
readonly sendSelectionToInteractiveWindow: boolean;
readonly normalizeSelectionForInteractiveWindow: boolean;
readonly markdownRegularExpression: string;
readonly codeRegularExpression: string;
readonly errorBackgroundColor: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class CodeExecutionHelper extends CodeExecutionHelperBase {
}
return normalizedLines;
} catch (ex) {
traceError(ex, 'Python: Failed to normalize code for execution in terminal');
traceError(ex, 'Python: Failed to normalize code for execution in Interactive Window');
return code;
}
}
Expand Down
27 changes: 22 additions & 5 deletions src/platform/terminals/codeExecution/codeExecutionHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export class CodeExecutionHelperBase implements ICodeExecutionHelper {
this.applicationShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
}

public async normalizeLines(_code: string, _resource?: Uri): Promise<string> {
throw Error('Not Implemented');
public async normalizeLines(code: string, _resource?: Uri): Promise<string> {
return code;
}

public async getFileToExecute(): Promise<Uri | undefined> {
Expand Down Expand Up @@ -61,7 +61,7 @@ export class CodeExecutionHelperBase implements ICodeExecutionHelper {
} else {
code = this.getMultiLineSelectionText(textEditor);
}
return code;
return this.dedentCode(code.trimEnd());
}

public async saveFileIfDirty(file: Uri): Promise<void> {
Expand All @@ -71,6 +71,23 @@ export class CodeExecutionHelperBase implements ICodeExecutionHelper {
}
}

private dedentCode(code: string) {
const lines = code.split('\n');
const firstNonEmptyLine = lines.find((line) => line.trim().length > 0);
if (firstNonEmptyLine) {
const leadingSpaces = firstNonEmptyLine.match(/^\s*/)![0];
return lines
.map((line) => {
if (line.startsWith(leadingSpaces)) {
return line.replace(leadingSpaces, '');
}
return line;
})
.join('\n');
}
return code;
}

private getSingleLineSelectionText(textEditor: TextEditor): string {
const selection = textEditor.selection;
const selectionRange = new Range(selection.start, selection.end);
Expand Down Expand Up @@ -152,8 +169,8 @@ export class CodeExecutionHelperBase implements ICodeExecutionHelper {
// if (m == 0):
// return n + 1
// ↑<------------------- To here (notice " + 1" is not selected)
if (selectionFirstLineText.trimLeft() === fullStartLineText.trimLeft()) {
return fullStartLineText + selectionText.substr(selectionFirstLineText.length);
if (selectionFirstLineText.trimStart() === fullStartLineText.trimStart()) {
return fullStartLineText + selectionText.substring(selectionFirstLineText.length);
}

// If you are here then user has selected partial start and partial end lines:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as assert from 'assert';
import * as TypeMoq from 'typemoq';
import { CodeExecutionHelperBase } from './codeExecutionHelper';
import { MockEditor } from '../../../test/datascience/mockTextEditor';
import { IServiceContainer } from '../../ioc/types';
import { Uri, Selection } from 'vscode';
import { MockDocumentManager } from '../../../test/datascience/mockDocumentManager';
import { createMockedDocument } from '../../../test/datascience/editor-integration/helpers';

function initializeMockTextEditor(inputText: string, selection: Selection): MockEditor {
const file = Uri.file('test.py');
const mockDocument = createMockedDocument(inputText, file, 1, true);
const mockTextEditor = new MockEditor(new MockDocumentManager(), mockDocument);
mockTextEditor.selection = selection;
return mockTextEditor;
}

const inputText = `print(1)
if (true):
print(2)
print(3)
print('''a multiline
string''')
`;

suite('Normalize selected text for execution', () => {
const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();

test('Normalize first line including newline', () => {
const editor = initializeMockTextEditor(inputText, new Selection(0, 0, 1, 0));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal('print(1)', text);
});

test('Normalize several lines', () => {
const editor = initializeMockTextEditor(inputText, new Selection(0, 0, 7, 0));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal(inputText.trimEnd(), text);
});

test('Normalize indented lines', () => {
const editor = initializeMockTextEditor(inputText, new Selection(3, 0, 5, 0));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal('print(2)\nprint(3)', text);
});

test('Normalize indented lines but first line partially selected', () => {
const editor = initializeMockTextEditor(inputText, new Selection(3, 3, 5, 0));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal('print(2)\nprint(3)', text);
});

test('Normalize single indented line', () => {
const editor = initializeMockTextEditor(inputText, new Selection(3, 4, 3, 12));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal('print(2)', text);
});

test('Normalize indented line including leading newline', () => {
const editor = initializeMockTextEditor(inputText, new Selection(3, 12, 4, 12));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal('\nprint(3)', text);
});

test('Normalize a multi-line string', () => {
const editor = initializeMockTextEditor(inputText, new Selection(5, 0, 7, 0));
const helper = new CodeExecutionHelperBase(serviceContainer.object);
const text = helper.getSelectedTextToExecute(editor);
assert.equal("print('''a multiline\nstring''')", text);
});
});
5 changes: 2 additions & 3 deletions src/test/datascience/mockTextEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@ import {
} from 'vscode';

import { noop } from '../../platform/common/utils/misc';
import { MockDocument } from './mockDocument';
import { MockDocumentManager } from './mockDocumentManager';

class MockEditorEdit implements TextEditorEdit {
constructor(
private _documentManager: MockDocumentManager,
private _document: MockDocument
private _document: TextDocument
) {}

public replace(location: Selection | Range | Position, value: string): void {
Expand Down Expand Up @@ -59,7 +58,7 @@ export class MockEditor implements TextEditor {

constructor(
private _documentManager: MockDocumentManager,
private _document: MockDocument
private _document: TextDocument
) {
this.selection = new Selection(0, 0, 0, 0);
this._revealCallback = noop;
Expand Down

0 comments on commit 450e4d0

Please sign in to comment.