compilerArgs entries fix (#143)

* Send different compilerArgs entries for bundled switches or pairs of switch-value if separated by space as a shell would see them

* Use a shell script to parse compiler arguments that are sent to CppTools.

* Fix running script with space in path. Use vscode.workspace.workspaceFolders instead of vscode.workspace.rootPath

src/configuration.ts

@@ -578,7 +578,7 @@ export function getBuildLogForConfiguration(configuration: string | undefined):
         logger.message('Found build log path setting "' + configurationBuildLog + '" defined for configuration "' + configuration);
 
         if (!path.isAbsolute(configurationBuildLog)) {
-            configurationBuildLog = path.join(vscode.workspace.rootPath || "", configurationBuildLog);
+            configurationBuildLog = path.join(util.getWorkspaceRoot(), configurationBuildLog);
             logger.message('Resolving build log path to "' + configurationBuildLog + '"');
         }
 

src/cpptools.ts

@@ -102,7 +102,7 @@ export class CppConfigurationProvider implements cpp.CustomConfigurationProvider
     }
 
     public async provideFolderBrowseConfiguration(_uri: vscode.Uri): Promise<cpp.WorkspaceBrowseConfiguration> {
-        if (_uri.fsPath !== vscode.workspace.rootPath) {
+        if (_uri.fsPath !== util.getWorkspaceRoot()) {
             logger.message("Makefile Tools supports single root for now.");
         }
 

src/extension.ts

@@ -322,7 +322,19 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
     // Delete the extension log file, if exists
     let extensionLog : string | undefined = configuration.getExtensionLog();
     if (extensionLog && util.checkFileExistsSync(extensionLog)) {
-        fs.unlinkSync(extensionLog);
+        util.deleteFileSync(extensionLog);
+    }
+
+    // Delete the script that is created by this extension in the temporary folder
+    // with the purpose of spliting a compilation command fragment into switch arguments
+    // as the shell sees them. See more about this script in parser.ts, parseAnySwitchFromToolArguments.
+    // We need to delete this file occasionally to ensure that the extension will not use indefinitely
+    // an eventual old version, especially because for performance reasons we don't create this file
+    // every time we use it (the configure process creates it every time it's not found on disk).
+    // Deleting this script here ensures that every new VSCode instance will operate on up to date script functionality.
+    let parseCompilerArgsScript: string = util.parseCompilerArgsScriptFile();
+    if (util.checkFileExistsSync(parseCompilerArgsScript)) {
+        util.deleteFileSync(parseCompilerArgsScript);
     }
 
     // Read configuration info from settings

src/launch.ts

@@ -52,7 +52,7 @@ export class Launcher implements vscode.Disposable {
         if (launchConfiguration) {
             return launchConfiguration.cwd;
         } else {
-            return vscode.workspace.rootPath || "";
+            return util.getWorkspaceRoot();
         }
     }
 

src/make.ts

@@ -314,7 +314,7 @@ export async function doBuildTarget(progress: vscode.Progress<{}>, target: strin
             logger.messageNoCR(result, "Normal");
         };
 
-        const result: util.SpawnProcessResult = await util.spawnChildProcess(configuration.getConfigurationMakeCommand(), makeArgs, vscode.workspace.rootPath || "", stdout, stderr);
+        const result: util.SpawnProcessResult = await util.spawnChildProcess(configuration.getConfigurationMakeCommand(), makeArgs, util.getWorkspaceRoot(), stdout, stderr);
         if (result.returnCode !== ConfigureBuildReturnCodeTypes.success) {
             logger.message(`Target ${target} failed to build.`);
         } else {
@@ -480,7 +480,7 @@ export async function generateParseContent(progress: vscode.Progress<{}>,
             }
         }, 5 * 1000);
 
-        const result: util.SpawnProcessResult = await util.spawnChildProcess(configuration.getConfigurationMakeCommand(), makeArgs, vscode.workspace.rootPath || "", stdout, stderr);
+        const result: util.SpawnProcessResult = await util.spawnChildProcess(configuration.getConfigurationMakeCommand(), makeArgs, util.getWorkspaceRoot(), stdout, stderr);
         clearInterval(timeout);
         let elapsedTime: number = util.elapsedTimeSince(startTime);
         logger.message(`Generating dry-run elapsed time: ${elapsedTime}`);
@@ -656,7 +656,7 @@ export async function runPreConfigureScript(progress: vscode.Progress<{}>, scrip
             logger.messageNoCR(result, "Normal");
         };
 
-        const result: util.SpawnProcessResult = await util.spawnChildProcess(runCommand, scriptArgs, vscode.workspace.rootPath || "", stdout, stderr);
+        const result: util.SpawnProcessResult = await util.spawnChildProcess(runCommand, scriptArgs, util.getWorkspaceRoot(), stdout, stderr);
         if (result.returnCode === ConfigureBuildReturnCodeTypes.success) {
             if (someErr) {
                 // Depending how the preconfigure scripts (and any inner called sub-scripts) are written,

src/parser.ts

@@ -141,7 +141,7 @@ export async function preprocessDryRunOutput(cancel: vscode.CancellationToken, d
     // Sometimes the ending of lines ends up being a mix and match of \n and \r\n.
     // Make it uniform to \n to ease other processing later.
     preprocessTasks.push(function (): void {
-        preprocessedDryRunOutputStr = preprocessedDryRunOutputStr.replace(/\\r\\n/mg, "\n");
+        preprocessedDryRunOutputStr = preprocessedDryRunOutputStr.replace(/\r\n/mg, "\n");
     });
 
     // Some compiler/linker commands are split on multiple lines.
@@ -364,21 +364,32 @@ async function parseLineAsTool(
 // The result is passed to IntelliSense custom configuration provider as compilerArgs.
 // excludeArgs helps with narrowing down the search, when we know for sure that we're not
 // interested in some switches. For example, -D, -I, -FI, -include, -std are treated separately.
-function parseAnySwitchFromToolArguments(args: string, excludeArgs: string[]): string[] {
+// Once we identified what looks to be the switches in the given command line, for each region
+// between two consecutive switches we let the shell parse it into arguments via a script invocation
+// (instead of us using other parser helpers in this module) to be in sync with how CppTools
+// expects the compiler arguments to be passed in.
+async function parseAnySwitchFromToolArguments(args: string, excludeArgs: string[]): Promise<string[]> {
     // Identify the non value part of the switch: prefix, switch name
     // and what may separate this from an eventual switch value
     let switches: string[] = [];
     let regExpStr: string = "(^|\\s+)(--|-" +
-                            // On Win32 allow '/' as switch prefix as well,
+        // On Win32 allow '/' as switch prefix as well,
-                            // otherwise it conflicts with path character
+        // otherwise it conflicts with path character
-                            (process.platform === "win32" ? "|\\/" : "") +
+        (process.platform === "win32" ? "|\\/" : "") +
-                            ")([a-zA-Z0-9_]+)";
+        ")([a-zA-Z0-9_]+)";
     let regexp: RegExp = RegExp(regExpStr, "mg");
     let match1: RegExpExecArray | null;
     let match2: RegExpExecArray | null;
     let index1: number = -1;
     let index2: number = -1;
 
+    // This contains all the compilation command fragments in between two different consecutive switches
+    // (except the ones we plan to ignore, specified by excludeArgs).
+    // Once this function is done concatenating into compilerArgRegions,
+    // we call the compiler args parsing script once for the whole list of regions
+    // (as opposed to invoking it for each fragment separately).
+    let compilerArgRegions: string = "";
+
     // With every loop iteration we need 2 switch matches so that we analyze the text
     // that is between them. If the current match is the last one, then we will analyze
     // everything until the end of line.
@@ -414,32 +425,75 @@ function parseAnySwitchFromToolArguments(args: string, excludeArgs: string[]): s
         }
 
         if (!exclude) {
-            // The other parser helpers differ from this one by the fact that they know
+            compilerArgRegions += partialArgs;
-            // what switch they are looking for. This helper first identifies anything
-            // that looks like a switch and then calls parseMultipleSwitchFromToolArguments
-            // which knows how to parse complex scenarios of spaces, quotes and other characters.
-            // Don't allow parseMultipleSwitchFromToolArguments to remove surrounding quotes for switch values.
-            let swiValues: string[] = parseMultipleSwitchFromToolArguments(partialArgs, swi, false);
-
-            // If no values are found, it means the switch has simple syntax.
-            // Add this to the array.
-            if (swiValues.length === 0) {
-                swiValues.push(swi);
-            }
-
-            swiValues.forEach(value => {
-                // The end of the current switch value
-                let index3: number = partialArgs.indexOf(value) + value.length;
-                let finalSwitch: string = partialArgs.substring(0, index3);
-
-                finalSwitch = finalSwitch.trim();
-                switches.push(finalSwitch);
-            });
         }
 
         match1 = match2;
     }
 
+    let parseCompilerArgsScriptFile: string = util.parseCompilerArgsScriptFile();
+    let parseCompilerArgsScriptContent: string;
+    if (process.platform === "win32") {
+        // There is a potential problem with the windows version of the script:
+        // A fragment like "-sw1,-sw2,-sw3" gets split by comma and a fragment like
+        // "-SwDef=Val" is split by equal. Opened GitHub issue
+        // https://github.com/microsoft/vscode-makefile-tools/issues/149.
+        // For now, these scenarios don't happen on windows (the comma syntax is linux only
+        // and the equal syntax is encountered for defines and c/c++ standard which are parsed
+        // separately, not via this script invocation).
+        parseCompilerArgsScriptContent = `@echo off\r\n`;
+        parseCompilerArgsScriptContent += `for %%i in (%*) do echo %%i \r\n`;
+    } else {
+        parseCompilerArgsScriptContent = `#!/bin/bash \n`;
+        parseCompilerArgsScriptContent += `while (( "$#" )); do \n`;
+        parseCompilerArgsScriptContent += `  echo $1 \n`;
+        parseCompilerArgsScriptContent += `  shift \n`;
+        parseCompilerArgsScriptContent += `done \n`;
+    }
+
+    // Create the script only when not found. The extension makes sure to delete it regularly
+    // (at project load time).
+    if (!util.checkFileExistsSync(parseCompilerArgsScriptFile)) {
+        util.writeFile(parseCompilerArgsScriptFile, parseCompilerArgsScriptContent);
+    }
+
+    let scriptArgs: string[] = [];
+    let runCommand: string;
+    if (process.platform === 'win32') {
+        runCommand = "cmd";
+        scriptArgs.push("/c");
+        scriptArgs.push(`""${parseCompilerArgsScriptFile}" ${compilerArgRegions}"`);
+    } else {
+        runCommand = "/bin/bash";
+        scriptArgs.push("-c");
+        scriptArgs.push(`"source '${parseCompilerArgsScriptFile}' ${compilerArgRegions}"`);
+    }
+
+    try {
+        let stdout: any = (result: string): void => {
+            let results: string[] = result.replace(/\r\n/mg, "\n").split("\n");
+            // In case of concatenated separators, the shell sees different empty arguments
+            // which we can remove (most common is more spaces not being seen as a single space).
+            results.forEach(res => {
+                if (res !== "") {
+                    switches.push(res.trim());
+                }
+            });
+        };
+
+        let stderr: any = (result: string): void => {
+            logger.messageNoCR(`Error while running the compiler args parser script '${parseCompilerArgsScriptFile}'` +
+                `for regions "${compilerArgRegions}": "${result}"`, "Normal");
+        };
+
+        const result: util.SpawnProcessResult = await util.spawnChildProcess(runCommand, scriptArgs, util.getWorkspaceRoot(), stdout, stderr);
+        if (result.returnCode !== 0) {
+            logger.messageNoCR(`The compiler args parser script '${parseCompilerArgsScriptFile}' failed with error code ${result.returnCode}`, "Normal");
+        }
+    } catch (error) {
+        logger.message(error);
+    }
+
     return switches;
 }
 
@@ -797,7 +851,7 @@ export async function parseCustomConfigProvider(cancel: vscode.CancellationToken
 
     // Current path starts with workspace root and can be modified
     // with prompt commands like cd, cd-, pushd/popd or with -C make switch
-    let currentPath: string = vscode.workspace.rootPath || "";
+    let currentPath: string = util.getWorkspaceRoot();
     let currentPathHistory: string[] = [currentPath];
 
     // Read the dry-run output line by line, searching for compilers and directory changing commands
@@ -806,6 +860,7 @@ export async function parseCustomConfigProvider(cancel: vscode.CancellationToken
     let numberOfLines: number = dryRunOutputLines.length;
     let index: number = 0;
     let done: boolean = false;
+
     async function doParsingChunk(): Promise<void> {
         let chunkIndex: number = 0;
         while (index < numberOfLines && chunkIndex <= chunkSize) {
@@ -833,7 +888,7 @@ export async function parseCustomConfigProvider(cancel: vscode.CancellationToken
                 // Exclude switches that are being processed separately (I, FI, include, D, std)
                 // and switches that don't affect IntelliSense but are causing errors.
                 let compilerArgs: string[] = [];
-                compilerArgs = parseAnySwitchFromToolArguments(compilerTool.arguments, ["I", "FI", "include", "D", "std", "MF"]);
+                compilerArgs = await parseAnySwitchFromToolArguments(compilerTool.arguments, ["I", "FI", "include", "D", "std", "MF"]);
 
                 // Parse and log the includes, forced includes and the defines
                 let includes: string[] = parseMultipleSwitchFromToolArguments(compilerTool.arguments, 'I');
@@ -954,7 +1009,7 @@ export async function parseLaunchConfigurations(cancel: vscode.CancellationToken
 
     // Current path starts with workspace root and can be modified
     // with prompt commands like cd, cd-, pushd/popd or with -C make switch
-    let currentPath: string = vscode.workspace.rootPath || "";
+    let currentPath: string = util.getWorkspaceRoot();
     let currentPathHistory: string[] = [currentPath];
 
     // array of full path executables built by this makefile
@@ -1156,7 +1211,7 @@ export async function parseLaunchConfigurations(cancel: vscode.CancellationToken
     // Reset the current path since we are going to analyze path transitions again
     // with this second pass through the dry-run output lines,
     // while building the launch custom provider data.
-    currentPath = vscode.workspace.rootPath || "";
+    currentPath = util.getWorkspaceRoot();
     currentPathHistory = [currentPath];
 
     // Since an executable can be called without its extension,

src/util.ts

@@ -77,7 +77,25 @@ export function tmpDir(): string {
     }
 }
 
-// Evaluate whether a string looks like a path or not,
+// Returns the full path to a temporary script generated by the extension
+// and used to parse any additional compiler switches that need to be sent to CppTools.
+export function parseCompilerArgsScriptFile(): string {
+    let scriptFile: string = path.join(tmpDir(), "parseCompilerArgs");
+
+    if (process.platform === "win32") {
+        scriptFile += ".bat";
+    } else {
+        scriptFile += ".sh";
+    }
+
+    return scriptFile;
+}
+
+export function getWorkspaceRoot(): string {
+    return vscode.workspace.workspaceFolders ? vscode.workspace.workspaceFolders[0].uri.fsPath : "";
+}
+
+ // Evaluate whether a string looks like a path or not,
 // without using fs.stat, since dry-run may output tools
 // that are not found yet at certain locations,
 // without running the prep targets that would copy them there
@@ -165,7 +183,7 @@ export async function killTree(progress: vscode.Progress<{}>, pid: number): Prom
     };
 
     try {
-        const result: SpawnProcessResult = await spawnChildProcess('pgrep', ['-P', pid.toString()], vscode.workspace.rootPath || "", stdout);
+        const result: SpawnProcessResult = await spawnChildProcess('pgrep', ['-P', pid.toString()], getWorkspaceRoot(), stdout);
         if (!!stdoutStr.length) {
             children = stdoutStr.split('\n').map((line: string) => Number.parseInt(line));
 
@@ -451,7 +469,7 @@ export function reportDryRunError(dryrunOutputFile: string): void {
 // Helper to make paths absolute until the extension handles variables expansion.
 export function resolvePathToRoot(relPath: string): string {
     if (!path.isAbsolute(relPath)) {
-        return path.join(vscode.workspace.rootPath || "", relPath);
+        return path.join(getWorkspaceRoot(), relPath);
     }
 
     return relPath;