🐛 Fixes microsoft#700 replace back slashes in fie paths with forward slashes (microsoft#707)

Fixes microsoft#700

Author: DonJayamanne

src/client/common/extensions.ts

@@ -20,6 +20,11 @@ interface String {
  * E.g. if an argument contains a space, then it will be enclosed within double quotes.
  */
  toCommandArgument(): string;
+ /**
+ * Appropriately formats a a file path so it can be used as an argument for a command in a shell.
+ * E.g. if an argument contains a space, then it will be enclosed within double quotes.
+ */
+ fileToCommandArgument(): string;
 }
 
 /**
@@ -47,5 +52,16 @@ String.prototype.toCommandArgument = function (this: string): string {
  if (!this) {
  return this;
  }
- return (this.indexOf(' ') > 0 && !this.startsWith('"') && !this.endsWith('"')) ? `"${this}"` : this.toString();
+ return (this.indexOf(' ') >= 0 && !this.startsWith('"') && !this.endsWith('"')) ? `"${this}"` : this.toString();
+};
+
+/**
+ * Appropriately formats a a file path so it can be used as an argument for a command in a shell.
+ * E.g. if an argument contains a space, then it will be enclosed within double quotes.
+ */
+String.prototype.fileToCommandArgument = function (this: string): string {
+ if (!this) {
+ return this;
+ }
+ return this.toCommandArgument().replace(/\\/g, '/');
 };

src/client/common/terminal/helper.ts

@@ -68,7 +68,7 @@ export class TerminalHelper implements ITerminalHelper {
  public buildCommandForTerminal(terminalShellType: TerminalShellType, command: string, args: string[]) {
  const isPowershell = terminalShellType === TerminalShellType.powershell || terminalShellType === TerminalShellType.powershellCore;
  const commandPrefix = isPowershell ? '& ' : '';
- return `${commandPrefix}${command.toCommandArgument()} ${args.join(' ')}`.trim();
+ return `${commandPrefix}${command.fileToCommandArgument()} ${args.join(' ')}`.trim();
  }
  public async getEnvironmentActivationCommands(terminalShellType: TerminalShellType, resource?: Uri): Promise<string[] | undefined> {
  const settings = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource);

src/client/terminals/codeExecution/djangoShellCodeExecution.ts

@@ -40,7 +40,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
  const workspaceRoot = workspaceUri ? workspaceUri.uri.fsPath : defaultWorkspace;
  const managePyPath = workspaceRoot.length === 0 ? 'manage.py' : path.join(workspaceRoot, 'manage.py');
 
- args.push(managePyPath.toCommandArgument());
+ args.push(managePyPath.fileToCommandArgument());
  args.push('shell');
  return { command, args };
  }

src/client/terminals/codeExecution/terminalCodeExecution.ts

@@ -34,7 +34,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
  const command = this.platformService.isWindows ? pythonSettings.pythonPath.replace(/\\/g, '/') : pythonSettings.pythonPath;
  const launchArgs = pythonSettings.terminal.launchArgs;
 
- await this.getTerminalService(file).sendCommand(command, launchArgs.concat(file.fsPath.toCommandArgument()));
+ await this.getTerminalService(file).sendCommand(command, launchArgs.concat(file.fsPath.fileToCommandArgument()));
  }
 
  public async execute(code: string, resource?: Uri): Promise<void> {
@@ -47,7 +47,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
  }
  public async initializeRepl(resource?: Uri) {
  if (this.replActive && await this.replActive!) {
- this._terminalService!.show();
+ await this._terminalService!.show();
  return;
  }
  this.replActive = new Promise<boolean>(async resolve => {

src/test/common/extensions.test.ts

@@ -0,0 +1,42 @@
+import { expect } from 'chai';
+import '../../client/common/extensions';
+
+// Defines a Mocha test suite to group tests of similar kind together
+suite('String Extensions', () => {
+ test('Should return empty string for empty arg', () => {
+ const argTotest = '';
+ expect(argTotest.toCommandArgument()).to.be.equal('');
+ });
+ test('Should quote an empty space', () => {
+ const argTotest = ' ';
+ expect(argTotest.toCommandArgument()).to.be.equal('" "');
+ });
+ test('Should not quote command arguments without spaces', () => {
+ const argTotest = 'one.two.three';
+ expect(argTotest.toCommandArgument()).to.be.equal(argTotest);
+ });
+ test('Should quote command arguments with spaces', () => {
+ const argTotest = 'one two three';
+ expect(argTotest.toCommandArgument()).to.be.equal(`"${argTotest}"`);
+ });
+ test('Should return empty string for empty path', () => {
+ const fileToTest = '';
+ expect(fileToTest.fileToCommandArgument()).to.be.equal('');
+ });
+ test('Should not quote file argument without spaces', () => {
+ const fileToTest = 'users/test/one';
+ expect(fileToTest.fileToCommandArgument()).to.be.equal(fileToTest);
+ });
+ test('Should quote file argument with spaces', () => {
+ const fileToTest = 'one two three';
+ expect(fileToTest.fileToCommandArgument()).to.be.equal(`"${fileToTest}"`);
+ });
+ test('Should replace all back slashes with forward slashes (irrespective of OS)', () => {
+ const fileToTest = 'c:\\users\\user\\conda\\scripts\\python.exe';
+ expect(fileToTest.fileToCommandArgument()).to.be.equal(fileToTest.replace(/\\/g, '/'));
+ });
+ test('Should replace all back slashes with forward slashes (irrespective of OS) and quoted when file has spaces', () => {
+ const fileToTest = 'c:\\users\\user namne\\conda path\\scripts\\python.exe';
+ expect(fileToTest.fileToCommandArgument()).to.be.equal(`"${fileToTest.replace(/\\/g, '/')}"`);
+ });
+});

src/test/common/terminals/activation.bash.test.ts

@@ -5,6 +5,7 @@ import { expect } from 'chai';
 import * as path from 'path';
 import * as TypeMoq from 'typemoq';
 import { EnumEx } from '../../../client/common/enumUtils';
+import '../../../client/common/extensions';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { Bash } from '../../../client/common/terminal/environmentActivationProviders/bash';
 import { TerminalShellType } from '../../../client/common/terminal/types';
@@ -13,7 +14,7 @@ import { IServiceContainer } from '../../../client/ioc/types';
 
 // tslint:disable-next-line:max-func-body-length
 suite('Terminal Environment Activation (bash)', () => {
- ['usr/bin/python', 'usr/bin/env with spaces/env more/python'].forEach(pythonPath => {
+ ['usr/bin/python', 'usr/bin/env with spaces/env more/python', 'c:\\users\\windows paths\\conda\\python.exe'].forEach(pythonPath => {
  const hasSpaces = pythonPath.indexOf(' ') > 0;
  const suiteTitle = hasSpaces ? 'and there are spaces in the script file (pythonpath),' : 'and there are no spaces in the script file (pythonpath),';
  suite(suiteTitle, () => {
@@ -83,8 +84,7 @@ suite('Terminal Environment Activation (bash)', () => {
  // Ensure the path is quoted if it contains any spaces.
  // Ensure it contains the name of the environment as an argument to the script file.
 
- const quotedScriptFile = pathToScriptFile.indexOf(' ') > 0 ? `"${pathToScriptFile}"` : pathToScriptFile;
- expect(command).to.be.deep.equal([`source ${quotedScriptFile}`.trim()], 'Invalid command');
+ expect(command).to.be.deep.equal([`source ${pathToScriptFile.fileToCommandArgument()}`.trim()], 'Invalid command');
  } else {
  expect(command).to.be.equal(undefined, 'Command should be undefined');
  }