Fix running on indented code. (microsoft#6051)

* Fix running on indented code. * Switch to using ICodeExecutionHelper * Fix functional test failure * Review feedback

Author: rchiodo

news/2 Fixes/5983.md

@@ -0,0 +1 @@
+Allow selection and running of indented code in the python interactive window.

package-lock.json

@@ -1317,7 +1317,7 @@
  },
  "python.dataScience.autoPreviewNotebooksInInteractivePane": {
  "type": "boolean",
- "default": true,
+ "default": false,
  "description": "When opening ipynb files, automatically preview the contents in the Python Interactive window.",
  "scope": "resource"
  },

package.json

@@ -10,6 +10,7 @@ import { IConfigurationService, IDataScienceSettings, ILogger } from '../../comm
 import * as localize from '../../common/utils/localize';
 import { noop } from '../../common/utils/misc';
 import { captureTelemetry } from '../../telemetry';
+import { ICodeExecutionHelper } from '../../terminals/types';
 import { generateCellRanges } from '../cellFactory';
 import { Commands, Telemetry } from '../constants';
 import { JupyterInstallError } from '../jupyter/jupyterInstallError';
@@ -29,7 +30,10 @@ export class CodeWatcher implements ICodeWatcher {
  @inject(IHistoryProvider) private historyProvider : IHistoryProvider,
  @inject(IFileSystem) private fileSystem: IFileSystem,
  @inject(IConfigurationService) private configService: IConfigurationService,
- @inject(IDocumentManager) private documentManager : IDocumentManager) {}
+ @inject(IDocumentManager) private documentManager : IDocumentManager,
+ @inject(ICodeExecutionHelper) private executionHelper: ICodeExecutionHelper
+ ) {
+ }
 
  public setDocument(document: TextDocument) {
  this.document = document;
@@ -165,19 +169,17 @@ export class CodeWatcher implements ICodeWatcher {
  this.fileSystem.arePathsSame(activeEditor.document.fileName, this.document.fileName)) {
 
  // Get just the text of the selection or the current line if none
- let code: string;
- if (activeEditor.selection.start.line === activeEditor.selection.end.line &&
- activeEditor.selection.start.character === activeEditor.selection.end.character) {
- const line = this.document.lineAt(activeEditor.selection.start.line);
- code = line.text;
- } else {
- code = this.document.getText(new Range(activeEditor.selection.start, activeEditor.selection.end));
+ const codeToExecute = await this.executionHelper.getSelectedTextToExecute(activeEditor);
+ if (!codeToExecute) {
+ return ;
  }
-
- if (code && code.trim().length) {
- const activeHistory = await this.historyProvider.getOrCreateActive();
- await activeHistory.addCode(code, this.getFileName(), activeEditor.selection.start.line, activeEditor);
+ const normalizedCode = await this.executionHelper.normalizeLines(codeToExecute!);
+ if (!normalizedCode || normalizedCode.trim().length === 0) {
+ return;
  }
+
+ const activeHistory = await this.historyProvider.getOrCreateActive();
+ await activeHistory.addCode(normalizedCode, this.getFileName(), activeEditor.selection.start.line, activeEditor);
  }
  }
 

src/client/datascience/editor-integration/codewatcher.ts

@@ -194,6 +194,8 @@ import {
 import { IPipEnvServiceHelper, IPythonInPathCommandProvider } from '../../client/interpreter/locators/types';
 import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs';
 import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types';
+import { CodeExecutionHelper } from '../../client/terminals/codeExecution/helper';
+import { ICodeExecutionHelper } from '../../client/terminals/types';
 import { IVsCodeApi } from '../../datascience-ui/react-common/postOffice';
 import { MockAutoSelectionService } from '../mocks/autoSelector';
 import { UnitTestIocContainer } from '../testing/serviceRegistry';
@@ -309,6 +311,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
  this.serviceManager.addSingleton<IPythonInPathCommandProvider>(IPythonInPathCommandProvider, PythonInPathCommandProvider);
  this.serviceManager.addSingleton<IEnvironmentActivationService>(IEnvironmentActivationService, EnvironmentActivationService);
  this.serviceManager.add<ICodeWatcher>(ICodeWatcher, CodeWatcher);
+ this.serviceManager.add<ICodeExecutionHelper>(ICodeExecutionHelper, CodeExecutionHelper);
  this.serviceManager.add<IDataScienceCommandListener>(IDataScienceCommandListener, HistoryCommandListener);
  this.serviceManager.addSingleton<IJupyterVariables>(IJupyterVariables, JupyterVariables);
 

src/test/datascience/dataScienceIocContainer.ts

@@ -16,6 +16,7 @@ import { DataScienceCodeLensProvider } from '../../../client/datascience/editor-
 import { CodeWatcher } from '../../../client/datascience/editor-integration/codewatcher';
 import { ICodeWatcher, IHistory, IHistoryProvider } from '../../../client/datascience/types';
 import { IServiceContainer } from '../../../client/ioc/types';
+import { ICodeExecutionHelper } from '../../../client/terminals/types';
 import { MockAutoSelectionService } from '../../mocks/autoSelector';
 import { createDocument } from './helpers';
 
@@ -32,6 +33,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
  let fileSystem: TypeMoq.IMock<IFileSystem>;
  let configService: TypeMoq.IMock<IConfigurationService>;
  let serviceContainer : TypeMoq.IMock<IServiceContainer>;
+ let helper: TypeMoq.IMock<ICodeExecutionHelper>;
  let tokenSource : CancellationTokenSource;
  const pythonSettings = new class extends PythonSettings {
  public fireChangeEvent() {
@@ -49,6 +51,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
  textEditor = TypeMoq.Mock.ofType<TextEditor>();
  fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
  configService = TypeMoq.Mock.ofType<IConfigurationService>();
+ helper = TypeMoq.Mock.ofType<ICodeExecutionHelper>();
 
  // Setup default settings
  pythonSettings.datascience = {
@@ -77,7 +80,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
 
  // Setup the service container to return code watchers
  serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
- serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICodeWatcher))).returns(() => new CodeWatcher(appShell.object, logger.object, historyProvider.object, fileSystem.object, configService.object, documentManager.object));
+ serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ICodeWatcher))).returns(() => new CodeWatcher(appShell.object, logger.object, historyProvider.object, fileSystem.object, configService.object, documentManager.object, helper.object));
 
  // Setup our active history instance
  historyProvider.setup(h => h.getOrCreateActive()).returns(() => Promise.resolve(activeHistory.object));
@@ -91,7 +94,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
  // Setup config service
  configService.setup(c => c.getSettings()).returns(() => pythonSettings);
 
- codeWatcher = new CodeWatcher(appShell.object, logger.object, historyProvider.object, fileSystem.object, configService.object, documentManager.object);
+ codeWatcher = new CodeWatcher(appShell.object, logger.object, historyProvider.object, fileSystem.object, configService.object, documentManager.object, helper.object);
  });
 
  function createTypeMoq<T>(tag: string): TypeMoq.IMock<T> {
@@ -570,6 +573,10 @@ testing2`;
  const document = createDocument(inputText, fileName, version, TypeMoq.Times.atLeastOnce());
 
  codeWatcher.setDocument(document.object);
+ helper.setup(h => h.getSelectedTextToExecute(TypeMoq.It.is((ed: TextEditor) => {
+ return textEditor.object === ed;
+ }))).returns(() => Promise.resolve('testing2'));
+ helper.setup(h => h.normalizeLines(TypeMoq.It.isAny())).returns(() => Promise.resolve('testing2'));
 
  // Set up our expected calls to add code
  activeHistory.setup(h => h.addCode(TypeMoq.It.isValue('testing2'),
@@ -591,34 +598,6 @@ testing2`;
  document.verifyAll();
  });
 
- test('Test the RunCurrentCell command outside of a cell', async () => {
- const fileName = 'test.py';
- const version = 1;
- const inputText =
-`testing1
-#%%
-testing2`;
- const document = createDocument(inputText, fileName, version, TypeMoq.Times.atLeastOnce());
-
- codeWatcher.setDocument(document.object);
-
- // We don't want to ever call add code here
- activeHistory.setup(h => h.addCode(TypeMoq.It.isAny(),
- TypeMoq.It.isAny(),
- TypeMoq.It.isAny(),
- TypeMoq.It.isAny())).verifiable(TypeMoq.Times.never());
-
- // For this test we need to set up a document selection point
- textEditor.setup(te => te.selection).returns(() => new Selection(0, 0, 0, 0));
-
- // Try our RunCell command with the first selection point
- await codeWatcher.runCurrentCell();
-
- // Verify function calls
- activeHistory.verifyAll();
- document.verifyAll();
- });
-
  test('Test the RunCellAndAdvance command with next cell', async () => {
  const fileName = 'test.py';
  const version = 1;

src/test/datascience/editor-integration/codewatcher.unit.test.ts

@@ -40,7 +40,7 @@ export function createDocument(inputText: string, fileName: string, fileVersion:
  document.setup(d => d.getText(TypeMoq.It.isAny())).returns((r: Range) => {
  let results = '';
  if (r) {
- for (let line = r.start.line; line <= r.end.line; line += 1) {
+ for (let line = r.start.line; line <= r.end.line && line < inputLines.length; line += 1) {
  const startIndex = line === r.start.line ? r.start.character : 0;
  const endIndex = line === r.end.line ? r.end.character : inputLines[line].length - 1;
  results += inputLines[line].slice(startIndex, endIndex + 1);