Enhancements to running code in a terminal

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`.

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);
     }
@@ -64,6 +65,6 @@ export class CodeExecutionManager implements ICodeExecutionManager {
             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

@@ -11,7 +11,7 @@ import { ICodeExecutionHelper } from '../types';
 
 @injectable()
 export class CodeExecutionHelper implements ICodeExecutionHelper {
-    constructor( @inject(IDocumentManager) private documentManager: IDocumentManager,
+    constructor(@inject(IDocumentManager) private documentManager: IDocumentManager,
         @inject(IApplicationShell) private applicationShell: IApplicationShell) {
 
     }
@@ -35,6 +35,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 +56,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

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

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(() => () => 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');

src/test/terminals/codeExecution/helper.test.ts

@@ -73,6 +73,45 @@ suite('Terminal - Code Execution Helper', () => {
         expect(uri).to.be.deep.equal(expectedUri);
     });
 
+    test('Returns file uri even if saving fails', async () => {
+        document.setup(doc => doc.isUntitled).returns(() => false);
+        document.setup(doc => doc.isDirty).returns(() => true);
+        document.setup(doc => doc.languageId).returns(() => PythonLanguage.language);
+        document.setup(doc => doc.save()).returns(() => Promise.resolve(false));
+        const expectedUri = Uri.file('one.py');
+        document.setup(doc => doc.uri).returns(() => expectedUri);
+        documentManager.setup(doc => doc.activeTextEditor).returns(() => editor.object);
+
+        const uri = await helper.getFileToExecute();
+        expect(uri).to.be.deep.equal(expectedUri);
+    });
+
+    test('Dirty files are saved', async () => {
+        document.setup(doc => doc.isUntitled).returns(() => false);
+        document.setup(doc => doc.isDirty).returns(() => true);
+        document.setup(doc => doc.languageId).returns(() => PythonLanguage.language);
+        const expectedUri = Uri.file('one.py');
+        document.setup(doc => doc.uri).returns(() => expectedUri);
+        documentManager.setup(doc => doc.activeTextEditor).returns(() => editor.object);
+
+        const uri = await helper.getFileToExecute();
+        expect(uri).to.be.deep.equal(expectedUri);
+        document.verify(doc => doc.save(), TypeMoq.Times.once());
+    });
+
+    test('Non-Dirty files are not-saved', async () => {
+        document.setup(doc => doc.isUntitled).returns(() => false);
+        document.setup(doc => doc.isDirty).returns(() => false);
+        document.setup(doc => doc.languageId).returns(() => PythonLanguage.language);
+        const expectedUri = Uri.file('one.py');
+        document.setup(doc => doc.uri).returns(() => expectedUri);
+        documentManager.setup(doc => doc.activeTextEditor).returns(() => editor.object);
+
+        const uri = await helper.getFileToExecute();
+        expect(uri).to.be.deep.equal(expectedUri);
+        document.verify(doc => doc.save(), TypeMoq.Times.never());
+    });
+
     test('Returns current line if nothing is selected', async () => {
         const lineContents = 'Line Contents';
         editor.setup(e => e.selection).returns(() => new Selection(3, 0, 3, 0));
@@ -94,4 +133,37 @@ suite('Terminal - Code Execution Helper', () => {
         const content = await helper.getSelectedTextToExecute(editor.object);
         expect(content).to.be.equal('3.0.10.5');
     });
+
+    test('saveFileIfDirty will not fail if file is not opened', async () => {
+        documentManager.setup(d => d.textDocuments).returns(() => []).verifiable(TypeMoq.Times.once());
+
+        await helper.saveFileIfDirty(Uri.file(`${__filename}.py`));
+        documentManager.verifyAll();
+    });
+
+    test('File will be saved if file is dirty', async () => {
+        documentManager.setup(d => d.textDocuments).returns(() => [document.object]).verifiable(TypeMoq.Times.once());
+        document.setup(doc => doc.isUntitled).returns(() => false);
+        document.setup(doc => doc.isDirty).returns(() => true);
+        document.setup(doc => doc.languageId).returns(() => PythonLanguage.language);
+        const expectedUri = Uri.file('one.py');
+        document.setup(doc => doc.uri).returns(() => expectedUri);
+
+        await helper.saveFileIfDirty(expectedUri);
+        documentManager.verifyAll();
+        document.verify(doc => doc.save(), TypeMoq.Times.once());
+    });
+
+    test('File will be not saved if file is not dirty', async () => {
+        documentManager.setup(d => d.textDocuments).returns(() => [document.object]).verifiable(TypeMoq.Times.once());
+        document.setup(doc => doc.isUntitled).returns(() => false);
+        document.setup(doc => doc.isDirty).returns(() => false);
+        document.setup(doc => doc.languageId).returns(() => PythonLanguage.language);
+        const expectedUri = Uri.file('one.py');
+        document.setup(doc => doc.uri).returns(() => expectedUri);
+
+        await helper.saveFileIfDirty(expectedUri);
+        documentManager.verifyAll();
+        document.verify(doc => doc.save(), TypeMoq.Times.never());
+    });
 });