Enhancements to running code in a terminal (#1432)

Fixes #1207
Fixes #1316
Fixes #1349
Fixes #259

news/1 Enhancements/1207.md

@@ -0,0 +1 @@
+Remove empty spaces from the selected text of the active editor when executing in a terminal.

news/1 Enhancements/1316.md

@@ -0,0 +1 @@
+Save the python file before running it in the terminal using the command/menu `Run Python File in Terminal`.

news/1 Enhancements/1349.md

@@ -0,0 +1 @@
+Add `Ctrl+Enter` keyboard shortcut for `Run Selection/Line in Python Terminal`.

news/2 Fixes/259.md

@@ -0,0 +1 @@
+Add blank lines to seprate blocks of indented code (function defs, classes, and the like) to ensure the code can be run within a Python interactive prompt.

package.json

@@ -94,6 +94,12 @@
                 "path": "./snippets/python.json"
             }
         ],
+        "keybindings":[
+            {
+                "command": "python.execSelectionInTerminal",
+                "key": "ctrl+enter"
+            }
+        ],
         "commands": [
             {
                 "command": "python.sortImports",

src/client/common/configSettings.ts

@@ -37,7 +37,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
     public formatting?: IFormattingSettings;
     public autoComplete?: IAutoCompleteSettings;
     public unitTest?: IUnitTestSettings;
-    public terminal?: ITerminalSettings;
+    public terminal!: ITerminalSettings;
     public sortImports?: ISortImportSettings;
     public workspaceSymbols?: IWorkspaceSymbolSettings;
     public disableInstallationChecks = false;

src/client/common/types.ts

@@ -106,7 +106,7 @@ export interface IPythonSettings {
     readonly formatting?: IFormattingSettings;
     readonly unitTest?: IUnitTestSettings;
     readonly autoComplete?: IAutoCompleteSettings;
-    readonly terminal?: ITerminalSettings;
+    readonly terminal: ITerminalSettings;
     readonly sortImports?: ISortImportSettings;
     readonly workspaceSymbols?: IWorkspaceSymbolSettings;
     readonly envFile: string;

src/client/terminals/codeExecution/codeExecutionManager.ts

@@ -35,6 +35,7 @@ export class CodeExecutionManager implements ICodeExecutionManager {
         if (!fileToExecute) {
             return;
         }
+        await codeExecutionHelper.saveFileIfDirty(fileToExecute);
         const executionService = this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService, 'standard');
         await executionService.executeFile(fileToExecute);
     }
@@ -59,11 +60,11 @@ export class CodeExecutionManager implements ICodeExecutionManager {
         }
         const codeExecutionHelper = this.serviceContainer.get<ICodeExecutionHelper>(ICodeExecutionHelper);
         const codeToExecute = await codeExecutionHelper.getSelectedTextToExecute(activeEditor!);
-        const normalizedCode = codeExecutionHelper.normalizeLines(codeToExecute!);
+        const normalizedCode = await codeExecutionHelper.normalizeLines(codeToExecute!);
         if (!normalizedCode || normalizedCode.trim().length === 0) {
             return;
         }
 
-        await executionService.execute(codeToExecute!, activeEditor!.document.uri);
+        await executionService.execute(normalizedCode, activeEditor!.document.uri);
     }
 }

src/client/terminals/codeExecution/djangoShellCodeExecution.ts

@@ -32,7 +32,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
     public getReplCommandArgs(resource?: Uri): { command: string; args: string[] } {
         const pythonSettings = this.configurationService.getSettings(resource);
         const command = this.platformService.isWindows ? pythonSettings.pythonPath.replace(/\\/g, '/') : pythonSettings.pythonPath;
-        const args = pythonSettings.terminal!.launchArgs.slice();
+        const args = pythonSettings.terminal.launchArgs.slice();
 
         const workspaceUri = resource ? this.workspace.getWorkspaceFolder(resource) : undefined;
         const defaultWorkspace = Array.isArray(this.workspace.workspaceFolders) && this.workspace.workspaceFolders.length > 0 ? this.workspace.workspaceFolders[0].uri.fsPath : '';

src/client/terminals/codeExecution/helper.ts

@@ -2,23 +2,34 @@
 // Licensed under the MIT License.
 
 import { inject, injectable } from 'inversify';
-import { EOL } from 'os';
 import { Range, TextEditor, Uri } from 'vscode';
 import { IApplicationShell, IDocumentManager } from '../../common/application/types';
 import { PythonLanguage } from '../../common/constants';
 import '../../common/extensions';
+import { IServiceContainer } from '../../ioc/types';
 import { ICodeExecutionHelper } from '../types';
 
 @injectable()
 export class CodeExecutionHelper implements ICodeExecutionHelper {
-    constructor( @inject(IDocumentManager) private documentManager: IDocumentManager,
-        @inject(IApplicationShell) private applicationShell: IApplicationShell) {
-
+    private readonly documentManager: IDocumentManager;
+    private readonly applicationShell: IApplicationShell;
+    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        this.documentManager = serviceContainer.get<IDocumentManager>(IDocumentManager);
+        this.applicationShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
     }
-    public normalizeLines(code: string): string {
-        const codeLines = code.splitLines({ trim: false, removeEmptyEntries: false });
-        const codeLinesWithoutEmptyLines = codeLines.filter(line => line.trim().length > 0);
-        return codeLinesWithoutEmptyLines.join(EOL);
+    public async normalizeLines(code: string, resource?: Uri): Promise<string> {
+        try {
+            if (code.trim().length === 0) {
+                return '';
+            }
+            const regex = /(\n)([ \t]*\r?\n)([ \t]+\S+)/gm;
+            return code.replace(regex, (_, a, b, c) => {
+                return `${a}${c}`;
+            });
+        } catch (ex) {
+            console.error(ex, 'Python: Failed to normalize code for execution in terminal');
+            return code;
+        }
     }
 
     public async getFileToExecute(): Promise<Uri | undefined> {
@@ -35,6 +46,9 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
             this.applicationShell.showErrorMessage('The active file is not a Python source file');
             return;
         }
+        if (activeEditor.document.isDirty) {
+            await activeEditor.document.save();
+        }
         return activeEditor.document.uri;
     }
 
@@ -53,4 +67,10 @@ export class CodeExecutionHelper implements ICodeExecutionHelper {
         }
         return code;
     }
+    public async saveFileIfDirty(file: Uri): Promise<void> {
+        const docs = this.documentManager.textDocuments.filter(d => d.uri.path === file.path);
+        if (docs.length === 1 && docs[0].isDirty) {
+            await docs[0].save();
+        }
+    }
 }

src/client/terminals/codeExecution/terminalCodeExecution.ts

@@ -10,16 +10,15 @@ import { IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
-import { IConfigurationService } from '../../common/types';
-import { IDisposableRegistry } from '../../common/types';
+import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import { ICodeExecutionService } from '../../terminals/types';
 
 @injectable()
 export class TerminalCodeExecutionProvider implements ICodeExecutionService {
-    protected terminalTitle: string;
-    private _terminalService: ITerminalService;
+    protected terminalTitle!: string;
+    private _terminalService!: ITerminalService;
     private replActive?: Promise<boolean>;
-    constructor( @inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory,
+    constructor(@inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory,
         @inject(IConfigurationService) protected readonly configurationService: IConfigurationService,
         @inject(IWorkspaceService) protected readonly workspace: IWorkspaceService,
         @inject(IDisposableRegistry) protected readonly disposables: Disposable[],
@@ -60,7 +59,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
 
         await this.replActive;
     }
-    public getReplCommandArgs(resource?: Uri): { command: string, args: string[] } {
+    public getReplCommandArgs(resource?: Uri): { command: string; args: string[] } {
         const pythonSettings = this.configurationService.getSettings(resource);
         const command = this.platformService.isWindows ? pythonSettings.pythonPath.replace(/\\/g, '/') : pythonSettings.pythonPath;
         const args = pythonSettings.terminal.launchArgs.slice();

src/client/terminals/types.ts

@@ -14,8 +14,9 @@ export interface ICodeExecutionService {
 export const ICodeExecutionHelper = Symbol('ICodeExecutionHelper');
 
 export interface ICodeExecutionHelper {
-    normalizeLines(code: string): string;
+    normalizeLines(code: string): Promise<string>;
     getFileToExecute(): Promise<Uri | undefined>;
+    saveFileIfDirty(file: Uri): Promise<void>;
     getSelectedTextToExecute(textEditor: TextEditor): Promise<string | undefined>;
 }
 

src/test/pythonFiles/terminalExec/sample1_normalized.py

@@ -0,0 +1,22 @@
+# Sample block 1
+def square(x):
+    return x**2
+
+print('hello')
+# Sample block 2
+a = 2
+if a < 2:
+    print('less than 2')
+else:
+    print('more than 2')
+
+print('hello')
+
+# Sample block 3
+for i in range(5):
+    print(i)
+    print(i)
+    print(i)
+    print(i)
+
+print('complete')

src/test/pythonFiles/terminalExec/sample1_raw.py

@@ -0,0 +1,24 @@
+# Sample block 1
+def square(x):
+    return x**2
+
+print('hello')
+# Sample block 2
+a = 2
+if a < 2:
+    print('less than 2')
+else:
+    print('more than 2')
+
+print('hello')
+
+# Sample block 3
+for i in range(5):
+    print(i)
+
+    print(i)
+    print(i)
+
+    print(i)
+
+print('complete')

src/test/pythonFiles/terminalExec/sample2_normalized.py

@@ -0,0 +1,7 @@
+def add(x, y):
+    """Adds x to y"""
+    # Some comment
+    return x + y
+
+v = add(1, 7)
+print(v)

src/test/pythonFiles/terminalExec/sample2_raw.py

@@ -0,0 +1,8 @@
+def add(x, y):
+    """Adds x to y"""
+    # Some comment
+
+    return x + y
+
+v = add(1, 7)
+print(v)

src/test/pythonFiles/terminalExec/sample3_normalized.py

@@ -0,0 +1,4 @@
+if True:
+    print(1)
+    print(2)
+print(3)

src/test/pythonFiles/terminalExec/sample3_raw.py

@@ -0,0 +1,5 @@
+if True:
+    print(1)
+
+    print(2)
+print(3)

src/test/pythonFiles/terminalExec/sample4_normalized.py

@@ -0,0 +1,6 @@
+class pc(object):
+    def __init__(self, pcname, model):
+        self.pcname = pcname
+        self.model = model
+    def print_name(self):
+        print('Workstation name is', self.pcname, 'model is', self.model)

src/test/pythonFiles/terminalExec/sample4_raw.py

@@ -0,0 +1,7 @@
+class pc(object):
+    def __init__(self, pcname, model):
+        self.pcname = pcname
+        self.model = model
+
+    def print_name(self):
+        print('Workstation name is', self.pcname, 'model is', self.model)

src/test/pythonFiles/terminalExec/sample5_normalized.py

@@ -0,0 +1,8 @@
+for i in range(10):
+    print('a')
+    for j in range(5):
+        print('b')
+        print('b2')
+        for k in range(2):
+            print('c')
+    print('done with first loop')

src/test/pythonFiles/terminalExec/sample5_raw.py

@@ -0,0 +1,11 @@
+for i in range(10):
+    print('a')
+    for j in range(5):
+        print('b')
+
+        print('b2')
+
+        for k in range(2):
+            print('c')
+
+    print('done with first loop')

src/test/terminals/codeExecution/codeExecutionManager.test.ts

@@ -69,7 +69,7 @@ suite('Terminal - Code Execution Manager', () => {
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(ICodeExecutionHelper))).returns(() => helper.object);
 
         await commandHandler!();
-        helper.verify(async h => await h.getFileToExecute(), TypeMoq.Times.once());
+        helper.verify(async h => h.getFileToExecute(), TypeMoq.Times.once());
     });
 
     test('Ensure executeFileInterTerminal will use provided file', async () => {
@@ -96,8 +96,8 @@ suite('Terminal - Code Execution Manager', () => {
 
         const fileToExecute = Uri.file('x');
         await commandHandler!(fileToExecute);
-        helper.verify(async h => await h.getFileToExecute(), TypeMoq.Times.never());
-        executionService.verify(async e => await e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once());
+        helper.verify(async h => h.getFileToExecute(), TypeMoq.Times.never());
+        executionService.verify(async e => e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once());
     });
 
     test('Ensure executeFileInterTerminal will use active file', async () => {
@@ -119,12 +119,12 @@ suite('Terminal - Code Execution Manager', () => {
         const fileToExecute = Uri.file('x');
         const helper = TypeMoq.Mock.ofType<ICodeExecutionHelper>();
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(ICodeExecutionHelper))).returns(() => helper.object);
-        helper.setup(async h => await h.getFileToExecute()).returns(() => Promise.resolve(fileToExecute));
+        helper.setup(async h => h.getFileToExecute()).returns(() => Promise.resolve(fileToExecute));
         const executionService = TypeMoq.Mock.ofType<ICodeExecutionService>();
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(ICodeExecutionService), TypeMoq.It.isValue('standard'))).returns(() => executionService.object);
 
         await commandHandler!(fileToExecute);
-        executionService.verify(async e => await e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once());
+        executionService.verify(async e => e.executeFile(TypeMoq.It.isValue(fileToExecute)), TypeMoq.Times.once());
     });
 
     async function testExecutionOfSelectionWithoutAnyActiveDocument(commandId: string, executionSericeId: string) {
@@ -150,7 +150,7 @@ suite('Terminal - Code Execution Manager', () => {
         documentManager.setup(d => d.activeTextEditor).returns(() => undefined);
 
         await commandHandler!();
-        executionService.verify(async e => await e.execute(TypeMoq.It.isAny()), TypeMoq.Times.never());
+        executionService.verify(async e => e.execute(TypeMoq.It.isAny()), TypeMoq.Times.never());
     }
 
     test('Ensure executeSelectionInTerminal will do nothing if theres no active document', async () => {
@@ -186,7 +186,7 @@ suite('Terminal - Code Execution Manager', () => {
         documentManager.setup(d => d.activeTextEditor).returns(() => { return {} as any; });
 
         await commandHandler!();
-        executionService.verify(async e => await e.execute(TypeMoq.It.isAny()), TypeMoq.Times.never());
+        executionService.verify(async e => e.execute(TypeMoq.It.isAny()), TypeMoq.Times.never());
     }
 
     test('Ensure executeSelectionInTerminal will do nothing if no text is selected', async () => {
@@ -218,7 +218,7 @@ suite('Terminal - Code Execution Manager', () => {
         const helper = TypeMoq.Mock.ofType<ICodeExecutionHelper>();
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(ICodeExecutionHelper))).returns(() => helper.object);
         helper.setup(h => h.getSelectedTextToExecute).returns(() => () => Promise.resolve(textSelected));
-        helper.setup(h => h.normalizeLines).returns(() => () => textSelected);
+        helper.setup(h => h.normalizeLines).returns(() => () => Promise.resolve(textSelected)).verifiable(TypeMoq.Times.once());
         const executionService = TypeMoq.Mock.ofType<ICodeExecutionService>();
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(ICodeExecutionService), TypeMoq.It.isValue(executionServiceId))).returns(() => executionService.object);
         const document = TypeMoq.Mock.ofType<TextDocument>();
@@ -228,7 +228,8 @@ suite('Terminal - Code Execution Manager', () => {
         documentManager.setup(d => d.activeTextEditor).returns(() => activeEditor.object);
 
         await commandHandler!();
-        executionService.verify(async e => await e.execute(TypeMoq.It.isValue(textSelected), TypeMoq.It.isValue(activeDocumentUri)), TypeMoq.Times.once());
+        executionService.verify(async e => e.execute(TypeMoq.It.isValue(textSelected), TypeMoq.It.isValue(activeDocumentUri)), TypeMoq.Times.once());
+        helper.verifyAll();
     }
     test('Ensure executeSelectionInTerminal will normalize selected text and send it to the terminal', async () => {
         await testExecutionOfSelectionIsSentToTerminal(Commands.Exec_Selection_In_Terminal, 'standard');