Partial/full activation mode and multi root. Disable automatic config…

…ures. (#1295) * Partial/full activation mode and multi root. Disable automatic configures. * Improvements from code review feedback * Rephrase warning message. * experiment, will undo some things after seeing tests results * Fix one test baseline. Check if bringing back the sourceDirectory listeners and call to enableFullFeatureSet still causes Electron issue. Co-authored-by: Bob Brown <[email protected]>
microsoft · Jun 17, 2020 · d1539be · d1539be
1 parent 2dd1a84
commit d1539be
Show file tree

Hide file tree

Showing 11 changed files with 142 additions and 71 deletions.
diff --git a/package.json b/package.json
@@ -1249,6 +1249,12 @@
           "description": "%cmake-tools.configuration.cmake.configureOnOpen.description%",
           "scope": "resource"
         },
+        "cmake.configureOnEdit": {
+          "type": "boolean",
+          "default": true,
+          "description": "%cmake-tools.configuration.cmake.configureOnEdit.description%",
+          "scope": "resource"
+        },
         "cmake.skipConfigureIfCachePresent": {
           "type": "boolean",
           "default": null,

diff --git a/package.nls.json b/package.nls.json
@@ -99,6 +99,7 @@
     "cmake-tools.configuration.cmake.emscriptenSearchDirs.description": "Directories where Emscripten may be installed.",
     "cmake-tools.configuration.cmake.copyCompileCommands.description": "Copy compile_commands.json to this location after a successful configure.",
     "cmake-tools.configuration.cmake.configureOnOpen.description": "Automatically configure CMake project directories when they are opened.",
+    "cmake-tools.configuration.cmake.configureOnEdit.description": "Automatically configure CMake project directories when cmake.sourceDirectory or CMakeLists.txt content are saved.",
     "cmake-tools.configuration.cmake.skipConfigureIfCachePresent.description": "Skip over the configure process if cache is present.",
     "cmake-tools.configuration.cmake.cmakeCommunicationMode": "The protocol used to communicate between the extension and CMake",
     "cmake-tools.configuration.cmake.ignoreKitEnv.description": "Do not use the kit environment variables when running CMake commands.",

diff --git a/src/cmake-tools.ts b/src/cmake-tools.ts
@@ -9,7 +9,7 @@ import diagCollections from '@cmt/diagnostics/collections';
 import * as shlex from '@cmt/shlex';
 import {StateManager} from '@cmt/state';
 import {Strand} from '@cmt/strand';
-import {ProgressHandle, versionToString} from '@cmt/util';
+import {ProgressHandle, versionToString, lightNormalizePath} from '@cmt/util';
 import {DirectoryContext} from '@cmt/workspace';
 import * as path from 'path';
 import * as vscode from 'vscode';
@@ -271,8 +271,21 @@ export class CMakeTools implements vscode.Disposable, api.CMakeToolsAPI {
           const cmakeListsFile = await vscode.window.showOpenDialog(openOpts);
           if (cmakeListsFile) {
             const fullPathDir: string = path.parse(cmakeListsFile[0].fsPath).dir;
-            const relPathDir: string = path.relative(this.folder.uri.fsPath, fullPathDir);
-            vscode.workspace.getConfiguration('cmake').update("sourceDirectory", path.join("${workspaceFolder}", relPathDir));
+            const relPathDir: string = lightNormalizePath(path.relative(this.folder.uri.fsPath, fullPathDir));
+            const joinedPath = "${workspaceFolder}/".concat(relPathDir);
+            vscode.workspace.getConfiguration('cmake', this.folder.uri).update("sourceDirectory", joinedPath);
+            const drv = await this.getCMakeDriverInstance();
+            if (drv) {
+              drv.config.updatePartial({sourceDirectory: joinedPath});
+            } else {
+              const reloadWindowButton = localize('reload.window', 'Reload Window');
+              const reload = await vscode.window.showErrorMessage(localize('setting.sourceDirectory.failed.needs.reload.window',
+                    'Something went wrong while updating the cmake.sourceDirectory setting. Please run the "Reload window" command for the change to take effect.'),
+                    reloadWindowButton);
+              if (reload === reloadWindowButton) {
+                vscode.commands.executeCommand('workbench.action.reloadWindow');
+              }
+            }
           } else {
             fullFeatureSet = false;
           }
@@ -289,14 +302,14 @@ export class CMakeTools implements vscode.Disposable, api.CMakeToolsAPI {
         // Previously, the user decided to ignore the missing CMakeFiles.txt.
         // Since we are here in cmakePreConditionProblemHandler, for the case of CMakePreconditionProblems.MissingCMakeListsFile,
         // it means that there weren't yet any reasons to switch to full functionality,
-        // so keep enableFullFeatureSet as true.
+        // so keep enableFullFeatureSet as false.
         fullFeatureSet = false;
       }
 
       break;
     }
 
-    await enableFullFeatureSet(fullFeatureSet);
+    await enableFullFeatureSet(fullFeatureSet, this.folder);
   }
 
   /**
@@ -445,14 +458,27 @@ export class CMakeTools implements vscode.Disposable, api.CMakeToolsAPI {
 
     this.extensionContext.subscriptions.push(vscode.workspace.onDidSaveTextDocument(async td => {
       const str = td.uri.fsPath;
-      if (str.endsWith("CMakeLists.txt")) {
-        // Reset to a full activation mode.
-        // If required, the extension will switch back to partial activation mode
-        // after more analysis triggered by the below configure.
-        await enableFullFeatureSet(true);
-
-        log.debug(localize('cmakelists.save.trigger.reconfigure', "We are saving a CMakeLists.txt file, attempting automatic reconfigure..."));
-        await this.configure([], ConfigureType.Normal);
+      const sourceDirectory = await this.sourceDir;
+      if (str === path.join(sourceDirectory, "CMakeLists.txt")) {
+        // The configure process can determine correctly whether the features set activation
+        // should be full or partial, so there is no need to proactively enable full here,
+        // unless the automatic configure is disabled.
+        // If there is a configure or a build in progress, we should avoid setting full activation here,
+        // even if cmake.configureOnEdit is true, because this may overwrite a different decision
+        // that was done earlier by that ongoing configure process.
+        const drv = await this.getCMakeDriverInstance();
+        if (drv && !drv.configOrBuildInProgress()) {
+          if (drv.config.configureOnEdit) {
+            log.debug(localize('cmakelists.save.trigger.reconfigure', "Detected saving of CMakeLists.txt, attempting automatic reconfigure..."));
+            await this.configure([], ConfigureType.Normal);
+          } else {
+            await enableFullFeatureSet(true, this.folder);
+          }
+        } else {
+          log.warning(localize('cmakelists.save.could.not.reconfigure',
+           'Changes were detected in CMakeLists.txt but we could not reconfigure the project because another operation is already in progress.'));
+          log.debug(localize('needs.reconfigure', 'The project needs to be reconfigured so that the changes saved in CMakeLists.txt have effect.'));
+        }
       }
     }));
   }
@@ -678,11 +704,13 @@ export class CMakeTools implements vscode.Disposable, api.CMakeToolsAPI {
                   break;
                 }
                 if (retc === 0) {
-                  // Switch to full CMake Tools extension functionality:
-                  // visible status bar and commands in the pallette.
-                  await enableFullFeatureSet(true);
                   await this._refreshCompileDatabase(drv.expansionOptions);
                 }
+                if (retc !== 2) {
+                  // Partial activation mode is required only for -2 error code,
+                  // which represents a missing CMakeLists.txt
+                  await enableFullFeatureSet(true, this.folder);
+                }
                 await this._ctestController.reloadTests(drv);
                 this._onReconfiguredEmitter.fire();
                 return retc;

diff --git a/src/config.ts b/src/config.ts
@@ -95,6 +95,7 @@ export interface ExtensionConfigurationSettings {
   emscriptenSearchDirs: string[];
   copyCompileCommands: string|null;
   configureOnOpen: boolean|null;
+  configureOnEdit: boolean;
   skipConfigureIfCachePresent: boolean|null;
   useCMakeServer: boolean;
   cmakeCommunicationMode: CMakeCommunicationMode;
@@ -241,6 +242,8 @@ export class ConfigurationReader implements vscode.Disposable {
 
   get configureOnOpen() { return this.configData.configureOnOpen; }
 
+  get configureOnEdit() { return this.configData.configureOnEdit; }
+
   get skipConfigureIfCachePresent() { return this.configData.skipConfigureIfCachePresent; }
 
   get useCMakeServer(): boolean { return this.configData.useCMakeServer; }
@@ -323,6 +326,7 @@ export class ConfigurationReader implements vscode.Disposable {
     emscriptenSearchDirs: new vscode.EventEmitter<string[]>(),
     copyCompileCommands: new vscode.EventEmitter<string|null>(),
     configureOnOpen: new vscode.EventEmitter<boolean|null>(),
+    configureOnEdit: new vscode.EventEmitter<boolean>(),
     skipConfigureIfCachePresent: new vscode.EventEmitter<boolean|null>(),
     useCMakeServer: new vscode.EventEmitter<boolean>(),
     cmakeCommunicationMode: new vscode.EventEmitter<CMakeCommunicationMode>(),

diff --git a/src/drivers/cmfileapi-driver.ts b/src/drivers/cmfileapi-driver.ts
@@ -21,6 +21,7 @@ import rollbar from '@cmt/rollbar';
 import * as util from '@cmt/util';
 import * as path from 'path';
 import * as vscode from 'vscode';
+import * as ext from '@cmt/extension';
 
 import {NoGeneratorError} from './cms-driver';
 
@@ -118,6 +119,26 @@ export class CMakeFileApiDriver extends codemodel.CodeModelDriver {
       log.debug(`Reload CMake cache: ${this.cachePath} changed`);
       rollbar.invokeAsync('Reloading CMake Cache', () => this.updateCodeModel());
     });
+
+    this.config.onChange('sourceDirectory', async () => {
+      // The configure process can determine correctly whether the features set activation
+      // should be full or partial, so there is no need to proactively enable full here,
+      // unless the automatic configure is disabled.
+      // If there is a configure or a build in progress, we should avoid setting full activation here,
+      // even if cmake.configureOnEdit is true, because this may overwrite a different decision
+      // that was done earlier by that ongoing configure process.
+      if (!this.configOrBuildInProgress()) {
+        if (this.config.configureOnEdit) {
+          log.debug(localize('cmakelists.save.trigger.reconfigure', "Detected 'cmake.sourceDirectory' setting update, attempting automatic reconfigure..."));
+          await this.configure([]);
+        } else if (this.workspaceFolder) {
+          const folder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(this.workspaceFolder));
+          if (folder) {
+            await ext.enableFullFeatureSet(true, folder);
+          }
+        }
+      }
+    });
   }
 
   doConfigureSettingsChange() { this._needsReconfigure = true; }

diff --git a/src/drivers/cms-driver.ts b/src/drivers/cms-driver.ts
@@ -15,6 +15,7 @@ import * as proc from '@cmt/proc';
 import rollbar from '@cmt/rollbar';
 import { ConfigurationReader } from '@cmt/config';
 import * as nls from 'vscode-nls';
+import * as ext from '@cmt/extension';
 
 nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
 const localize: nls.LocalizeFunc = nls.loadMessageBundle();
@@ -319,7 +320,30 @@ export class CMakeServerClientDriver extends codemodel.CodeModelDriver {
     }
   }
 
-  protected async doInit(): Promise<void> { await this._restartClient(); }
+  protected async doInit(): Promise<void> {
+    await this._restartClient();
+
+
+    this.config.onChange('sourceDirectory', async () => {
+      // The configure process can determine correctly whether the features set activation
+      // should be full or partial, so there is no need to proactively enable full here,
+      // unless the automatic configure is disabled.
+      // If there is a configure or a build in progress, we should avoid setting full activation here,
+      // even if cmake.configureOnEdit is true, because this may overwrite a different decision
+      // that was done earlier by that ongoing configure process.
+      if (!this.configOrBuildInProgress()) {
+        if (this.config.configureOnEdit) {
+          log.debug(localize('cmakelists.save.trigger.reconfigure', "Detected 'cmake.sourceDirectory' setting update, attempting automatic reconfigure..."));
+          await this.configure([]);
+        } else if (this.workspaceFolder) {
+          const folder = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(this.workspaceFolder));
+          if (folder) {
+            await ext.enableFullFeatureSet(true, folder);
+          }
+        }
+      }
+    });
+  }
 
   static async create(cmake: CMakeExecutable, config: ConfigurationReader, kit: Kit|null, workspaceFolder: string | null, preconditionHandler: CMakePreconditionProblemSolver, preferredGenerators: CMakeGenerator[]): Promise<CMakeServerClientDriver> {
     return this.createDerived(new CMakeServerClientDriver(cmake, config, workspaceFolder, preconditionHandler), kit, preferredGenerators);

diff --git a/src/drivers/driver.ts b/src/drivers/driver.ts
@@ -593,6 +593,10 @@ export abstract class CMakeDriver implements vscode.Disposable {
 
   private buildRunning: boolean = false;
 
+  public configOrBuildInProgress() : boolean {
+    return this.configRunning || this.buildRunning;
+  }
+
   /**
    * Perform a clean configure. Deletes cached files before running the config
    * @param consumer The output consumer
@@ -631,7 +635,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
 
       const pre_check_ok = await this._beforeConfigureOrBuild();
       if (!pre_check_ok) {
-        return -1;
+        return -2;
       }
 
       const common_flags = ['--no-warn-unused-cli'].concat(extra_args, this.config.configureArgs);

diff --git a/src/extension.ts b/src/extension.ts
@@ -104,6 +104,13 @@ class ExtensionManager implements vscode.Disposable {
           this._setupSubscriptions();
         }
         await util.setContextValue(MULTI_ROOT_MODE_KEY, this._folders.isMultiRoot);
+
+        // Removing a workspace should trigger a re-evaluation of the partial/full activation mode
+        // of the extension, because the visibility depends on having at least one folder
+        // with valid CMakeLists.txt. If that one happens to be this, we need an opportunity
+        // to hide the commands and status bar.
+        this._enableFullFeatureSetOnWorkspace = false;
+        this.enableWorkspaceFoldersFullFeatureSet();
       }
 
       this._onDidChangeActiveTextEditorSub.dispose();
@@ -127,21 +134,6 @@ class ExtensionManager implements vscode.Disposable {
       }
       this._statusBar.setAutoSelectActiveFolder(v);
     });
-
-    this._workspaceConfig.onChange('sourceDirectory', async v => {
-      await this._onDidChangeActiveTextEditorSub.dispose();
-      if (v) {
-        this._onDidChangeActiveTextEditorSub = vscode.window.onDidChangeActiveTextEditor(e => this._onDidChangeActiveTextEditor(e), this);
-      } else {
-        this._onDidChangeActiveTextEditorSub = new DummyDisposable();
-      }
-
-      // Reset to a full activation mode.
-      // If required, the extension will switch back to partial activation mode
-      // after more analysis triggered by the below configure.
-      await enableFullFeatureSet(true);
-      await vscode.commands.executeCommand("cmake.configure");
-    });
   }
 
   private _onDidChangeActiveTextEditorSub: vscode.Disposable = new DummyDisposable();
@@ -182,30 +174,24 @@ class ExtensionManager implements vscode.Disposable {
     telemetry.logEvent('open', telemetryProperties);
   }
 
-  public getActiveFolderContext() : StateManager | undefined {
-    const activeFolder = this.getActiveFolder();
-    if (activeFolder) {
-      const extensionState = new StateManager(this.extensionContext, activeFolder);
-      return extensionState;
-    }
-
-    return undefined;
+  public getFolderContext(folder: vscode.WorkspaceFolder): StateManager {
+    return new StateManager(this.extensionContext, folder);
   }
 
   // Partial activation means that the CMake Tools commands are hidden
   // from the commands pallette and the status bar is not visible.
-  // The context variable "cmake:enableFullFeatureSet" is always equal
-  // to the state setting ignoreCMakeListsMissing.
-  // To have them both always in sync, cmake:enableFullFeatureSet is set
-  // by the getter and setter of ignoreCMakeListsMissing.
-  public enableFullFeatureSet(fullFeatureSet: boolean) {
-    const context = this.getActiveFolderContext();
-    if (context) {
-      context.ignoreCMakeListsMissing = !fullFeatureSet;
-      this._statusBar.setVisible(fullFeatureSet);
-    } else {
-      log.trace(localize('enableFullFeatureSet.no.active.folder', 'enableFullFeatureSet({0}) called but not active folder found.'), fullFeatureSet);
-    }
+  // The context variable "cmake:enableFullFeatureSet" (which controls
+  // all the cmake commands and UI elements) is set to true,
+  // if there is at least one folder with full features set enabled.
+  // We need to add this private _enableFullFeaturesSetOnWorkspace here
+  // because currently there is no way of reading a context variable
+  // like cmake:enableFullFeatureSet, to apply the OR operation on it.
+  private _enableFullFeatureSetOnWorkspace = false;
+  public enableFullFeatureSet(fullFeatureSet: boolean, folder: vscode.WorkspaceFolder) {
+    this.getFolderContext(folder).ignoreCMakeListsMissing = !fullFeatureSet;
+    this._enableFullFeatureSetOnWorkspace = this._enableFullFeatureSetOnWorkspace || fullFeatureSet;
+    util.setContextValue("cmake:enableFullFeatureSet", this._enableFullFeatureSetOnWorkspace);
+    this._statusBar.setVisible(this._enableFullFeatureSetOnWorkspace);
   }
 
   /**
@@ -222,13 +208,6 @@ class ExtensionManager implements vscode.Disposable {
    * The folder controller manages multiple instances. One per folder.
    */
   private readonly _folders = new CMakeToolsFolderController(this.extensionContext);
-  public getActiveFolder(): vscode.WorkspaceFolder | undefined {
-    if (!vscode.workspace.workspaceFolders || vscode.workspace.workspaceFolders.length === 0) {
-      return undefined;
-    }
-
-    return this._folders?.activeFolder?.cmakeTools.folder || vscode.workspace.workspaceFolders![0];
-  }
 
   /**
    * The status bar controller
@@ -959,6 +938,14 @@ class ExtensionManager implements vscode.Disposable {
     this._statusBar.hideDebugButton(shouldHide);
     await util.setContextValue(HIDE_DEBUG_COMMAND_KEY, shouldHide);
   }
+
+  // Helper that loops through all the workspace folders to enable full or partial feature set
+  // depending on their 'ignoreCMakeListsMissing' state variable.
+  enableWorkspaceFoldersFullFeatureSet() {
+    for (const cmtFolder of this._folders) {
+      this.enableFullFeatureSet(!this.getFolderContext(cmtFolder.folder)?.ignoreCMakeListsMissing, cmtFolder.folder);
+    }
+  }
 }
 
 /**
@@ -973,8 +960,8 @@ async function setup(context: vscode.ExtensionContext, progress: ProgressHandle)
   // Load a new extension manager
   const ext = _EXT_MANAGER = await ExtensionManager.create(context);
 
-  // Enable full or partial feature set, depending on state variable.
-  ext.enableFullFeatureSet(!ext.getActiveFolderContext()?.ignoreCMakeListsMissing);
+  // Enable full or partial feature set for each workspace folder, depending on their state variable.
+  ext.enableWorkspaceFoldersFullFeatureSet();
 
   // A register function that helps us bind the commands to the extension
   function register<K extends keyof ExtensionManager>(name: K) {
@@ -1145,8 +1132,8 @@ export async function activate(context: vscode.ExtensionContext) {
   // context.subscriptions.push(vscode.commands.registerCommand('cmake._extensionInstance', () => cmt));
 }
 
-export async function enableFullFeatureSet(fullFeatureSet: boolean) {
-    _EXT_MANAGER?.enableFullFeatureSet(fullFeatureSet);
+export async function enableFullFeatureSet(fullFeatureSet: boolean, folder: vscode.WorkspaceFolder) {
+    _EXT_MANAGER?.enableFullFeatureSet(fullFeatureSet, folder);
 }
 
 // this method is called when your extension is deactivated

diff --git a/src/state.ts b/src/state.ts
@@ -1,4 +1,3 @@
-import * as util from '@cmt/util';
 import * as vscode from 'vscode';
 
 /**
@@ -26,15 +25,11 @@ export class StateManager {
    * Whether the user chose to ignore the popup message about missing CMakeLists.txt
    * from the root folder, for a code base that is not fully activating CMake Tools.
    */
-  // Set the context var here in both getter and setter?
   get ignoreCMakeListsMissing(): boolean {
-    const ignore = this._get<boolean>('ignoreCMakeListsMissing') || false;
-    util.setContextValue("cmake:enableFullFeatureSet", !ignore);
-    return ignore;
+    return this._get<boolean>('ignoreCMakeListsMissing') || false;
   }
   set ignoreCMakeListsMissing(v: boolean) {
     this._update('ignoreCMakeListsMissing', v);
-    util.setContextValue("cmake:enableFullFeatureSet", !v);
   }
 
   /**