Fix installing missing tools (#293)

Author: KalitaAlexey

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: [] };
         }

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

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);
     }
 }

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

src/components/tools_installation/installator.ts

@@ -1,45 +1,49 @@
 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();
         });
     }
 
     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 = <string[]>missingTools;
-        logger.debug(`this.missingTools = ${JSON.stringify(this.missingTools)}`);
+        this._missingTools = <string[]>missingTools;
+        logger.debug(`this.missingTools = ${JSON.stringify(this._missingTools)}`);
     }
 }

src/extension.ts

@@ -408,7 +408,7 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
     }
     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<void> {
             await runInLegacyMode(
                 ctx,
                 configuration,
+                cargoInvocationManager,
                 rustSource,
                 rustup,
                 currentWorkingDirectoryManager,
@@ -469,6 +470,7 @@ export async function activate(ctx: ExtensionContext): Promise<void> {
 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,

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: ')
         );
     }