From 8e6422d3f22f35cac8c1fc4de340a2bcfe6456be Mon Sep 17 00:00:00 2001 From: Alexey Date: Wed, 21 Jun 2017 20:19:24 +0300 Subject: [PATCH] Fix installing missing tools (#293) --- src/CargoInvocationManager.ts | 6 +-- src/CommandLine.ts | 12 +++++ src/components/cargo/terminal_task_manager.ts | 10 ++-- src/components/configuration/Configuration.ts | 4 -- .../tools_installation/installator.ts | 52 +++++++++++-------- src/extension.ts | 5 +- src/legacy_mode_manager.ts | 5 ++ 7 files changed, 57 insertions(+), 37 deletions(-) diff --git a/src/CargoInvocationManager.ts b/src/CargoInvocationManager.ts index b090170..f3764e9 100644 --- a/src/CargoInvocationManager.ts +++ b/src/CargoInvocationManager.ts @@ -5,11 +5,9 @@ import { Rustup } from './components/configuration/Rustup'; * The class defines functions which can be used to get data required to invoke Cargo */ export class CargoInvocationManager { - private _configuration: Configuration; private _rustup: Rustup | undefined; - public constructor(configuration: Configuration, rustup: Rustup | undefined) { - this._configuration = configuration; + public constructor(rustup: Rustup | undefined) { this._rustup = rustup; } @@ -20,7 +18,7 @@ export class CargoInvocationManager { * understand that Cargo is requested. An example is running Cargo using rustup. */ public getExecutableAndArgs(): { executable: string, args: string[] } { - const userCargoPath = this._configuration.getCargoPath(); + const userCargoPath = Configuration.getPathConfigParameter('cargoPath'); if (userCargoPath) { return { executable: userCargoPath, args: [] }; } diff --git a/src/CommandLine.ts b/src/CommandLine.ts index 85a5ae6..3680fb1 100644 --- a/src/CommandLine.ts +++ b/src/CommandLine.ts @@ -76,6 +76,18 @@ export function escapeSpaces(s: string, shell: Shell): string { } } +/** + * Prepares the specified arguments to be passed to the specified shell and constructs the command + * from the arguments + * @param shell The shell in which the command will be executed + * @param args The arguments to prepare and construct the command from + * @return The command which is constructed from the specified arguments + */ +export function getCommandForArgs(shell: Shell, args: string[]): string { + args = args.map(a => escapeSpaces(a, shell)); + return args.join(' '); +} + /** * Creates a command to execute several statements one by one if the previous one is succeed * @param shell The shell which the command is going to be passed to diff --git a/src/components/cargo/terminal_task_manager.ts b/src/components/cargo/terminal_task_manager.ts index 0418083..00b0872 100644 --- a/src/components/cargo/terminal_task_manager.ts +++ b/src/components/cargo/terminal_task_manager.ts @@ -1,5 +1,6 @@ import { ExtensionContext, Terminal, window, workspace } from 'vscode'; -import { escapeSpaces, getCommandToSetEnvVar, parseShell } from '../../CommandLine'; +import { getCommandForArgs, getCommandToSetEnvVar, parseShell } + from '../../CommandLine'; import { Configuration } from '../configuration/Configuration'; export class TerminalTaskManager { @@ -53,13 +54,10 @@ export class TerminalTaskManager { } }; setEnvironmentVariables(); - cwd = escapeSpaces(cwd, shell); // Change the current directory to a specified directory - this._runningTerminal.sendText(`cd ${cwd}`); - executable = escapeSpaces(executable, shell); - args = args.map((arg) => escapeSpaces(arg, shell)); + this._runningTerminal.sendText(getCommandForArgs(shell, ['cd', cwd])); // Start a requested command - this._runningTerminal.sendText(`${executable} ${args.join(' ')}`); + this._runningTerminal.sendText(getCommandForArgs(shell, [executable, ...args])); this._runningTerminal.show(true); } } diff --git a/src/components/configuration/Configuration.ts b/src/components/configuration/Configuration.ts index da2e081..df851bc 100644 --- a/src/components/configuration/Configuration.ts +++ b/src/components/configuration/Configuration.ts @@ -206,10 +206,6 @@ export class Configuration { return Configuration.getPathConfigParameter('cargoCwd'); } - public getCargoPath(): string | undefined { - return Configuration.getPathConfigParameter('cargoPath'); - } - public getCargoHomePath(): string | undefined { const configPath = Configuration.getPathConfigParameter('cargoHomePath'); const envPath = Configuration.getPathEnvParameter('CARGO_HOME'); diff --git a/src/components/tools_installation/installator.ts b/src/components/tools_installation/installator.ts index 53bd085..a0bb9e2 100644 --- a/src/components/tools_installation/installator.ts +++ b/src/components/tools_installation/installator.ts @@ -1,28 +1,32 @@ import { existsSync } from 'fs'; import * as path from 'path'; import { ExtensionContext, commands, window, workspace } from 'vscode'; -import { getCommandToExecuteStatementsOneByOneIfPreviousIsSucceed, parseShell } +import { CargoInvocationManager } from '../../CargoInvocationManager'; +import { getCommandForArgs, getCommandToExecuteStatementsOneByOneIfPreviousIsSucceed, parseShell } from '../../CommandLine'; import { Configuration } from '../configuration/Configuration'; import { ChildLogger } from '../logging/child_logger'; import { MissingToolsStatusBarItem } from './missing_tools_status_bar_item'; export class Installator { - private configuration: Configuration; - private logger: ChildLogger; - private missingToolsStatusBarItem: MissingToolsStatusBarItem; - private missingTools: string[]; + private _configuration: Configuration; + private _cargoInvocationManager: CargoInvocationManager; + private _logger: ChildLogger; + private _missingToolsStatusBarItem: MissingToolsStatusBarItem; + private _missingTools: string[]; public constructor( context: ExtensionContext, configuration: Configuration, + cargoInvocationManager: CargoInvocationManager, logger: ChildLogger ) { - this.configuration = configuration; - this.logger = logger; + this._configuration = configuration; + this._cargoInvocationManager = cargoInvocationManager; + this._logger = logger; const installToolsCommandName = 'rust.install_missing_tools'; - this.missingToolsStatusBarItem = new MissingToolsStatusBarItem(context, installToolsCommandName); - this.missingTools = []; + this._missingToolsStatusBarItem = new MissingToolsStatusBarItem(context, installToolsCommandName); + this._missingTools = []; commands.registerCommand(installToolsCommandName, () => { this.offerToInstallMissingTools(); }); @@ -30,16 +34,16 @@ export class Installator { public addStatusBarItemIfSomeToolsAreMissing(): void { this.getMissingTools(); - if (this.missingTools.length === 0) { + if (this._missingTools.length === 0) { return; } - this.missingToolsStatusBarItem.show(); + this._missingToolsStatusBarItem.show(); } private offerToInstallMissingTools(): void { // Plurality is important. :') - const group = this.missingTools.length > 1 ? 'them' : 'it'; - const message = `You are missing ${this.missingTools.join(', ')}. Would you like to install ${group}?`; + const group = this._missingTools.length > 1 ? 'them' : 'it'; + const message = `You are missing ${this._missingTools.join(', ')}. Would you like to install ${group}?`; const option = { title: 'Install' }; window.showInformationMessage(message, option).then(selection => { if (selection !== option) { @@ -52,23 +56,27 @@ export class Installator { private installMissingTools(): void { const terminal = window.createTerminal('Rust tools installation'); // cargo install tool && cargo install another_tool - const cargoBinPath = this.configuration.getCargoPath(); + const { executable: cargoExecutable, args: cargoArgs } = this._cargoInvocationManager.getExecutableAndArgs(); const shell = parseShell(workspace.getConfiguration('terminal')['integrated']['shell']['windows']); - const statements = this.missingTools.map(tool => `${cargoBinPath} install ${tool}`); + + const statements = this._missingTools.map(tool => { + const args = [cargoExecutable, ...cargoArgs, 'install', tool]; + return getCommandForArgs(shell, args); + }); const command = getCommandToExecuteStatementsOneByOneIfPreviousIsSucceed(shell, statements); terminal.sendText(command); terminal.show(); - this.missingToolsStatusBarItem.hide(); + this._missingToolsStatusBarItem.hide(); } private getMissingTools(): void { - const logger = this.logger.createChildLogger('getMissingTools(): '); + const logger = this._logger.createChildLogger('getMissingTools(): '); const pathDirectories: string[] = (process.env.PATH || '').split(path.delimiter); logger.debug(`pathDirectories=${JSON.stringify(pathDirectories)}`); const tools: { [tool: string]: string | undefined } = { - 'racer': this.configuration.getPathToRacer(), - 'rustfmt': this.configuration.getRustfmtPath(), - 'rustsym': this.configuration.getRustsymPath() + 'racer': this._configuration.getPathToRacer(), + 'rustfmt': this._configuration.getRustfmtPath(), + 'rustsym': this._configuration.getRustsymPath() }; logger.debug(`tools=${JSON.stringify(tools)}`); const keys = Object.keys(tools); @@ -99,7 +107,7 @@ export class Installator { // The tool wasn't found, we should install it return tool; }).filter(tool => tool !== undefined); - this.missingTools = missingTools; - logger.debug(`this.missingTools = ${JSON.stringify(this.missingTools)}`); + this._missingTools = missingTools; + logger.debug(`this.missingTools = ${JSON.stringify(this._missingTools)}`); } } diff --git a/src/extension.ts b/src/extension.ts index 17bfa46..5da7393 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -408,7 +408,7 @@ export async function activate(ctx: ExtensionContext): Promise { } const rustSource = await RustSource.create(rustup); const configuration = new Configuration(logger.createChildLogger('Configuration: ')); - const cargoInvocationManager = new CargoInvocationManager(configuration, rustup); + const cargoInvocationManager = new CargoInvocationManager(rustup); const rlsConfiguration = await RlsConfiguration.create(rustup, rustSource); if (configuration.mode() === undefined) { // The current configuration does not contain any specified mode and hence we should try to @@ -455,6 +455,7 @@ export async function activate(ctx: ExtensionContext): Promise { await runInLegacyMode( ctx, configuration, + cargoInvocationManager, rustSource, rustup, currentWorkingDirectoryManager, @@ -469,6 +470,7 @@ export async function activate(ctx: ExtensionContext): Promise { async function runInLegacyMode( context: ExtensionContext, configuration: Configuration, + cargoInvocationManager: CargoInvocationManager, rustSource: RustSource, rustup: Rustup | undefined, currentWorkingDirectoryManager: CurrentWorkingDirectoryManager, @@ -477,6 +479,7 @@ async function runInLegacyMode( const legacyModeManager = await LegacyModeManager.create( context, configuration, + cargoInvocationManager, rustSource, rustup, currentWorkingDirectoryManager, diff --git a/src/legacy_mode_manager.ts b/src/legacy_mode_manager.ts index da33b40..0e71576 100644 --- a/src/legacy_mode_manager.ts +++ b/src/legacy_mode_manager.ts @@ -13,6 +13,7 @@ import { WorkspaceSymbolProvisionManager } from './components/symbol_provision/workspace_symbol_provision_manager'; import { Installator as MissingToolsInstallator } from './components/tools_installation/installator'; +import { CargoInvocationManager } from './CargoInvocationManager'; export class LegacyModeManager { private context: ExtensionContext; @@ -26,6 +27,7 @@ export class LegacyModeManager { public static async create( context: ExtensionContext, configuration: Configuration, + cargoInvocationManager: CargoInvocationManager, rustSource: RustSource, rustup: Rustup | undefined, currentWorkingDirectoryManager: CurrentWorkingDirectoryManager, @@ -35,6 +37,7 @@ export class LegacyModeManager { return new LegacyModeManager( context, configuration, + cargoInvocationManager, rustSource, rustup, currentWorkingDirectoryManager, @@ -53,6 +56,7 @@ export class LegacyModeManager { private constructor( context: ExtensionContext, configuration: Configuration, + cargoInvocationManager: CargoInvocationManager, rustSource: RustSource, rustup: Rustup | undefined, currentWorkingDirectoryManager: CurrentWorkingDirectoryManager, @@ -78,6 +82,7 @@ export class LegacyModeManager { this.missingToolsInstallator = new MissingToolsInstallator( context, configuration, + cargoInvocationManager, logger.createChildLogger('MissingToolsInstallator: ') ); }