From 46263bdcafff0b68bfb1a723ab5e23450039d7dd Mon Sep 17 00:00:00 2001 From: Andreea Isac Date: Mon, 1 Mar 2021 04:33:36 -0800 Subject: [PATCH 1/5] Ensure kit selection quickPick doesn't show on non CMake projects --- package.json | 22 +++++++++--------- package.nls.json | 2 +- src/cmake-tools.ts | 1 + src/extension.ts | 56 +++++++++++++++++++++++++++++++++------------- src/state.ts | 22 ++++++++++++++++++ 5 files changed, 76 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index 5bb1e953c..ad4e499b3 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "onCommand:cmake.debugTarget", "onCommand:cmake.debugTargetAll", "onCommand:cmake.editCache", + "onCommand:cmake.editCacheUI", "onCommand:cmake.editKits", "onCommand:cmake.viewLog", "onCommand:cmake.install", @@ -297,13 +298,15 @@ "title": "%cmake-tools.command.cmake.cleanRebuildAll.title%" }, { - "command": "cmake.openConfiguration", - "title": "%cmake-tools.command.cmake.openConfiguration.title%", + "command": "cmake.editCacheUI", + "when": "cmake:enableFullFeatureSet", + "title": "%cmake-tools.command.cmake.editCacheUI.title%", "category": "CMake" }, { - "command": "cmake.outline.openConfiguration", - "title": "%cmake-tools.command.cmake.openConfiguration.title%" + "command": "cmake.outline.editCacheUI", + "when": "cmake:enableFullFeatureSet", + "title": "%cmake-tools.command.cmake.editCacheUI.title%" }, { "command": "cmake.ctest", @@ -391,7 +394,6 @@ { "command": "cmake.resetState", "title": "%cmake-tools.command.cmake.resetState.title%", - "when": "cmake:enableFullFeatureSet", "category": "CMake" }, { @@ -477,7 +479,6 @@ "command": "cmake.selectActiveFolder" }, { - "when": "cmake:enableFullFeatureSet", "command": "cmake.resetState" }, { @@ -544,7 +545,8 @@ "when": "cmake:multiRoot && cmake:enableFullFeatureSet" }, { - "command": "cmake.openConfiguration" + "command": "cmake.editCacheUI", + "when": "cmake:enableFullFeatureSet" }, { "when": "cmake:enableFullFeatureSet", @@ -635,7 +637,7 @@ "when": "never" }, { - "command": "cmake.outline.openConfiguration", + "command": "cmake.outline.editCacheUI", "when": "never" }, { @@ -707,8 +709,8 @@ "group": "1_cmakeOutline" }, { - "command": "cmake.outline.openConfiguration", - "when": "view == cmake.outline", + "command": "cmake.outline.editCacheUI", + "when": "view == cmake.outline && cmake:enableFullFeatureSet", "group": "1_cmakeOutline" } ], diff --git a/package.nls.json b/package.nls.json index 772da4afb..ab62845b1 100644 --- a/package.nls.json +++ b/package.nls.json @@ -18,7 +18,7 @@ "cmake-tools.command.cmake.setDefaultTarget.title": "Set Build Target", "cmake-tools.command.cmake.cleanConfigure.title": "Delete Cache and Reconfigure", "cmake-tools.command.cmake.cleanConfigureAll.title": "Delete Cache and Reconfigure All Projects", - "cmake-tools.command.cmake.openConfiguration.title": "Edit CMake Cache (UI)", + "cmake-tools.command.cmake.editCacheUI.title": "Edit CMake Cache (UI)", "cmake-tools.command.cmake.outline.cleanConfigure.title": "Clean Reconfigure", "cmake-tools.command.cmake.outline.cleanConfigureAll.title": "Clean Reconfigure All Projects", "cmake-tools.command.cmake.clean.title": "Clean", diff --git a/src/cmake-tools.ts b/src/cmake-tools.ts index f01439b52..38d459808 100644 --- a/src/cmake-tools.ts +++ b/src/cmake-tools.ts @@ -74,6 +74,7 @@ export enum ConfigureTrigger { commandCleanConfigure = "commandCleanConfigure", commandConfigureAll = "commandConfigureAll", commandCleanConfigureAll = "commandCleanConfigureAll", + kitSelection = "kitSelection", } /** diff --git a/src/extension.ts b/src/extension.ts index 0a53d5e41..48528a799 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -21,6 +21,7 @@ import { findCLCompilerPath, effectiveKitEnvironment, scanForKitsIfNeeded, + SpecialKits } from '@cmt/kit'; import {KitsController} from '@cmt/kitsController'; import * as logging from '@cmt/logging'; @@ -288,11 +289,12 @@ class ExtensionManager implements vscode.Disposable { /** * Ensure that there is an active kit for the current CMakeTools. + * Ask the user to select one depending on showQuickPick boolean. * * @returns `false` if there is not active CMakeTools, or it has no active kit * and the user cancelled the kit selection dialog. */ - private async _ensureActiveKit(cmt?: CMakeTools): Promise { + private async _ensureActiveKit(cmt?: CMakeTools, showQuickPick: boolean = true): Promise { if (!cmt) { cmt = this._folders.activeFolder?.cmakeTools; } @@ -304,8 +306,8 @@ class ExtensionManager implements vscode.Disposable { // We have an active kit. We're good. return true; } - // No kit? Ask the user what they want. - const did_choose_kit = await this.selectKit(cmt.folder); + // No kit? Ask the user what they want, if showQuickPick allows. + const did_choose_kit = showQuickPick && await this.selectKit(cmt.folder); if (!did_choose_kit && !cmt.activeKit) { // The user did not choose a kit and kit isn't set in other way such as setKitByName return false; @@ -346,6 +348,32 @@ class ExtensionManager implements vscode.Disposable { await telemetry.deactivate(); } + async configureInternal(trigger: ConfigureTrigger, cmt: CMakeTools): Promise { + // If there is no active kit defined for this project, set it temporarily to "unspecified". + // This is needed so that the kit selection quickPick is not shown from the very beginning. + // See more details in the comments of the state variable "userSelectedKit" (state.ts). + if (!await this._ensureActiveKit(cmt, false)) { + await cmt.setKit({name: SpecialKits.Unspecified}); + } + + const retc = await cmt.configureInternal(trigger, [], ConfigureType.Normal); + + // If the configure process did not identify that CMakeLists.txt is missing, + // we can continue with asking the user for a kit selection. + // If the user does not select a kit at this point, the current configure + // is still going to be based on the temporary "unspecified" that was set earlier, + // but since the workspace state did not record a user selection it will ask again + // during the next configure. + if (retc !== -2 && !cmt.workspaceContext.state.userSelectedKit) { + if (await this.selectKit(cmt.folder) && cmt.activeKit?.name !== SpecialKits.Unspecified) { + // Configure again if the user selected a different kit than our original temporary "unspecified". + if (cmt.activeKit?.name !== SpecialKits.Unspecified) { + await cmt.configureInternal(ConfigureTrigger.kitSelection, [], ConfigureType.Normal); + } + } + } + } + async _postWorkspaceOpen(info: CMakeToolsFolder) { const ws = info.folder; const cmt = info.cmakeTools; @@ -406,15 +434,12 @@ class ExtensionManager implements vscode.Disposable { rollbar.takePromise(localize('persist.config.on.open.setting', 'Persist config-on-open setting'), {}, persist_pr); should_configure = chosen.doConfigure; } + if (should_configure) { // We've opened a new workspace folder, and the user wants us to // configure it now. log.debug(localize('configuring.workspace.on.open', 'Configuring workspace on open {0}', ws.uri.toString())); - // Ensure that there is a kit. This is required for new instances. - if (!await this._ensureActiveKit(cmt)) { - return; - } - await cmt.configureInternal(ConfigureTrigger.configureOnOpen, [], ConfigureType.Normal); + await this.configureInternal(ConfigureTrigger.configureOnOpen, cmt); } else if (silentScanForKitsNeeded) { // This popup will show up the first time after deciding not to configure, if a version change has been detected // in the kits definition. This may happen during a CMake Tools extension upgrade. @@ -423,13 +448,13 @@ class ExtensionManager implements vscode.Disposable { const configureButtonMessage = localize('configure.now.button', 'Configure Now'); const result = await vscode.window.showWarningMessage(localize('configure.recommended', 'It is recommended to reconfigure after upgrading to a new kits definition.'), configureButtonMessage); if (result === configureButtonMessage) { - // Ensure that there is a kit. This is required for new instances. - if (!await this._ensureActiveKit(cmt)) { - return; - } - await cmt.configureInternal(ConfigureTrigger.buttonNewKitsDefinition, [], ConfigureType.Normal); + await this.configureInternal(ConfigureTrigger.buttonNewKitsDefinition, cmt); } } + + // Enable full or partial feature set for each workspace folder, depending on their state variable. + this.enableWorkspaceFoldersFullFeatureSet(); + this._updateCodeModel(info); } @@ -720,8 +745,10 @@ class ExtensionManager implements vscode.Disposable { } if (kitName) { + cmtFolder.cmakeTools.workspaceContext.state.userSelectedKit = true; return true; } + return false; } @@ -1101,9 +1128,6 @@ 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 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(name: K) { return vscode.commands.registerCommand(`cmake.${name}`, (...args: any[]) => { diff --git a/src/state.ts b/src/state.ts index 17a147ecb..0c144b1ef 100644 --- a/src/state.ts +++ b/src/state.ts @@ -32,6 +32,27 @@ export class StateManager { this._update('ignoreCMakeListsMissing', v); } + /** + * Whether the extension had an opportunity to ask the user to select a kit, + * since this project was created (or since the last state reset). + * This is to fix the unwanted kit selection quickPick that is shown also for non CMake projects. + * A configure cannot succeed if no kit is given. + * Also, whether CMakeLists.txt is missing or not cannot be accurately determined + * without variable expansion functionality, which depends on the initial stages of configure. + * To solve this "chicken and egg" situation, if we find a project without a kit selected, + * we temporarily set an "unspecified kit", so that configure continues. + * If during configure we find that the CMakeLists.txt is missing, the user sees the popup + * and also was not inconvenienced with the kit selection quickPick. + * If the configure finds a CMakeLists.txt, by reading this state variable we know + * whether the kit selection was postponed or not and we can show the quickPick accordingly. + */ + get userSelectedKit(): boolean { + return this._get('userSelectedKit') || false; + } + set userSelectedKit(v: boolean) { + this._update('userSelectedKit', v); + } + /** * The name of the workspace-local active kit. */ @@ -85,5 +106,6 @@ export class StateManager { this.defaultBuildTarget = null; this.activeKitName = null; this.ignoreCMakeListsMissing = false; + this.userSelectedKit = false; } } From f089724fa67c1d6c3f5116fb62eab88326b225e6 Mon Sep 17 00:00:00 2001 From: Andreea Isac Date: Tue, 2 Mar 2021 00:56:02 -0800 Subject: [PATCH 2/5] A new approach to stop asking for kits for a non CMake project --- src/extension.ts | 90 +++++++++++++++++++++++++++--------------------- src/state.ts | 22 ------------ 2 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index 48528a799..1f1826dbc 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -5,10 +5,12 @@ 'use strict'; import * as chokidar from 'chokidar'; +import {expandString, ExpansionVars} from './expand'; import * as path from 'path'; import * as vscode from 'vscode'; import * as cpt from 'vscode-cpptools'; import * as nls from 'vscode-nls'; +import paths from './paths'; import {CMakeCache} from '@cmt/cache'; import {CMakeTools, ConfigureType, ConfigureTrigger} from '@cmt/cmake-tools'; @@ -21,7 +23,6 @@ import { findCLCompilerPath, effectiveKitEnvironment, scanForKitsIfNeeded, - SpecialKits } from '@cmt/kit'; import {KitsController} from '@cmt/kitsController'; import * as logging from '@cmt/logging'; @@ -289,12 +290,11 @@ class ExtensionManager implements vscode.Disposable { /** * Ensure that there is an active kit for the current CMakeTools. - * Ask the user to select one depending on showQuickPick boolean. * * @returns `false` if there is not active CMakeTools, or it has no active kit * and the user cancelled the kit selection dialog. */ - private async _ensureActiveKit(cmt?: CMakeTools, showQuickPick: boolean = true): Promise { + private async _ensureActiveKit(cmt?: CMakeTools): Promise { if (!cmt) { cmt = this._folders.activeFolder?.cmakeTools; } @@ -307,7 +307,7 @@ class ExtensionManager implements vscode.Disposable { return true; } // No kit? Ask the user what they want, if showQuickPick allows. - const did_choose_kit = showQuickPick && await this.selectKit(cmt.folder); + const did_choose_kit = await this.selectKit(cmt.folder); if (!did_choose_kit && !cmt.activeKit) { // The user did not choose a kit and kit isn't set in other way such as setKitByName return false; @@ -349,29 +349,11 @@ class ExtensionManager implements vscode.Disposable { } async configureInternal(trigger: ConfigureTrigger, cmt: CMakeTools): Promise { - // If there is no active kit defined for this project, set it temporarily to "unspecified". - // This is needed so that the kit selection quickPick is not shown from the very beginning. - // See more details in the comments of the state variable "userSelectedKit" (state.ts). - if (!await this._ensureActiveKit(cmt, false)) { - await cmt.setKit({name: SpecialKits.Unspecified}); + if (!await this._ensureActiveKit(cmt)) { + return; } - const retc = await cmt.configureInternal(trigger, [], ConfigureType.Normal); - - // If the configure process did not identify that CMakeLists.txt is missing, - // we can continue with asking the user for a kit selection. - // If the user does not select a kit at this point, the current configure - // is still going to be based on the temporary "unspecified" that was set earlier, - // but since the workspace state did not record a user selection it will ask again - // during the next configure. - if (retc !== -2 && !cmt.workspaceContext.state.userSelectedKit) { - if (await this.selectKit(cmt.folder) && cmt.activeKit?.name !== SpecialKits.Unspecified) { - // Configure again if the user selected a different kit than our original temporary "unspecified". - if (cmt.activeKit?.name !== SpecialKits.Unspecified) { - await cmt.configureInternal(ConfigureTrigger.kitSelection, [], ConfigureType.Normal); - } - } - } + await cmt.configureInternal(trigger, [], ConfigureType.Normal); } async _postWorkspaceOpen(info: CMakeToolsFolder) { @@ -435,21 +417,50 @@ class ExtensionManager implements vscode.Disposable { should_configure = chosen.doConfigure; } - if (should_configure) { - // We've opened a new workspace folder, and the user wants us to - // configure it now. - log.debug(localize('configuring.workspace.on.open', 'Configuring workspace on open {0}', ws.uri.toString())); - await this.configureInternal(ConfigureTrigger.configureOnOpen, cmt); - } else if (silentScanForKitsNeeded) { - // This popup will show up the first time after deciding not to configure, if a version change has been detected - // in the kits definition. This may happen during a CMake Tools extension upgrade. - // The warning is emitted only once because scanForKitsIfNeeded returns true only once after such change, - // being tied to a global state variable. - const configureButtonMessage = localize('configure.now.button', 'Configure Now'); - const result = await vscode.window.showWarningMessage(localize('configure.recommended', 'It is recommended to reconfigure after upgrading to a new kits definition.'), configureButtonMessage); - if (result === configureButtonMessage) { - await this.configureInternal(ConfigureTrigger.buttonNewKitsDefinition, cmt); + const optsVars: ExpansionVars = { + userHome: paths.userHome, + workspaceFolder: cmt.workspaceContext.folder.uri.fsPath, + workspaceFolderBasename: cmt.workspaceContext.folder.name, + workspaceRoot: cmt.workspaceContext.folder.uri.fsPath, + workspaceRootFolderName: cmt.workspaceContext.folder.name, + + // sourceDirectory cannot be defined based on any of the below variables. + buildKit: "", + buildType: "", + generator: "", + buildKitVendor: "", + buildKitTriple: "", + buildKitVersion: "", + buildKitHostOs: "", + buildKitTargetOs: "", + buildKitTargetArch: "", + buildKitVersionMajor: "", + buildKitVersionMinor: "", + workspaceHash: "", + }; + + // Don't configure if the current project is not CMake based (it doesn't have a CMakeLists.txt in the sourceDirectory). + const sourceDirectory: string = cmt.workspaceContext.config.sourceDirectory; + const expandedSourceDirectory: string = util.lightNormalizePath(await expandString(sourceDirectory, { vars: optsVars })); + if (await fs.exists(expandedSourceDirectory + "/CMakeLists.txt")) { + if (should_configure) { + // We've opened a new workspace folder, and the user wants us to + // configure it now. + log.debug(localize('configuring.workspace.on.open', 'Configuring workspace on open {0}', ws.uri.toString())); + await this.configureInternal(ConfigureTrigger.configureOnOpen, cmt); + } else if (silentScanForKitsNeeded) { + // This popup will show up the first time after deciding not to configure, if a version change has been detected + // in the kits definition. This may happen during a CMake Tools extension upgrade. + // The warning is emitted only once because scanForKitsIfNeeded returns true only once after such change, + // being tied to a global state variable. + const configureButtonMessage = localize('configure.now.button', 'Configure Now'); + const result = await vscode.window.showWarningMessage(localize('configure.recommended', 'It is recommended to reconfigure after upgrading to a new kits definition.'), configureButtonMessage); + if (result === configureButtonMessage) { + await this.configureInternal(ConfigureTrigger.buttonNewKitsDefinition, cmt); + } } + } else { + await enableFullFeatureSet(false, cmt.folder); } // Enable full or partial feature set for each workspace folder, depending on their state variable. @@ -745,7 +756,6 @@ class ExtensionManager implements vscode.Disposable { } if (kitName) { - cmtFolder.cmakeTools.workspaceContext.state.userSelectedKit = true; return true; } diff --git a/src/state.ts b/src/state.ts index 0c144b1ef..17a147ecb 100644 --- a/src/state.ts +++ b/src/state.ts @@ -32,27 +32,6 @@ export class StateManager { this._update('ignoreCMakeListsMissing', v); } - /** - * Whether the extension had an opportunity to ask the user to select a kit, - * since this project was created (or since the last state reset). - * This is to fix the unwanted kit selection quickPick that is shown also for non CMake projects. - * A configure cannot succeed if no kit is given. - * Also, whether CMakeLists.txt is missing or not cannot be accurately determined - * without variable expansion functionality, which depends on the initial stages of configure. - * To solve this "chicken and egg" situation, if we find a project without a kit selected, - * we temporarily set an "unspecified kit", so that configure continues. - * If during configure we find that the CMakeLists.txt is missing, the user sees the popup - * and also was not inconvenienced with the kit selection quickPick. - * If the configure finds a CMakeLists.txt, by reading this state variable we know - * whether the kit selection was postponed or not and we can show the quickPick accordingly. - */ - get userSelectedKit(): boolean { - return this._get('userSelectedKit') || false; - } - set userSelectedKit(v: boolean) { - this._update('userSelectedKit', v); - } - /** * The name of the workspace-local active kit. */ @@ -106,6 +85,5 @@ export class StateManager { this.defaultBuildTarget = null; this.activeKitName = null; this.ignoreCMakeListsMissing = false; - this.userSelectedKit = false; } } From d55ee31263a0cccc11a3c6e0bf2766bcb7edd520 Mon Sep 17 00:00:00 2001 From: Andreea Isac Date: Tue, 2 Mar 2021 08:53:28 -0800 Subject: [PATCH 3/5] Consider having CMakeLists.txt at the end of sourceDirectory --- src/cmake-tools.ts | 1 - src/extension.ts | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cmake-tools.ts b/src/cmake-tools.ts index 38d459808..f01439b52 100644 --- a/src/cmake-tools.ts +++ b/src/cmake-tools.ts @@ -74,7 +74,6 @@ export enum ConfigureTrigger { commandCleanConfigure = "commandCleanConfigure", commandConfigureAll = "commandConfigureAll", commandCleanConfigureAll = "commandCleanConfigureAll", - kitSelection = "kitSelection", } /** diff --git a/src/extension.ts b/src/extension.ts index 1f1826dbc..c066860ef 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -306,7 +306,7 @@ class ExtensionManager implements vscode.Disposable { // We have an active kit. We're good. return true; } - // No kit? Ask the user what they want, if showQuickPick allows. + // No kit? Ask the user what they want. const did_choose_kit = await this.selectKit(cmt.folder); if (!did_choose_kit && !cmt.activeKit) { // The user did not choose a kit and kit isn't set in other way such as setKitByName @@ -441,8 +441,11 @@ class ExtensionManager implements vscode.Disposable { // Don't configure if the current project is not CMake based (it doesn't have a CMakeLists.txt in the sourceDirectory). const sourceDirectory: string = cmt.workspaceContext.config.sourceDirectory; - const expandedSourceDirectory: string = util.lightNormalizePath(await expandString(sourceDirectory, { vars: optsVars })); - if (await fs.exists(expandedSourceDirectory + "/CMakeLists.txt")) { + let expandedSourceDirectory: string = util.lightNormalizePath(await expandString(sourceDirectory, { vars: optsVars })); + if (path.basename(expandedSourceDirectory).toLocaleLowerCase() !== "cmakelists.txt") { + expandedSourceDirectory = path.join(expandedSourceDirectory, "/CMakeLists.txt"); + } + if (await fs.exists(expandedSourceDirectory)) { if (should_configure) { // We've opened a new workspace folder, and the user wants us to // configure it now. From 53cb49636d00af917a10d0f6f785b804b52f8c71 Mon Sep 17 00:00:00 2001 From: Andreea Isac Date: Tue, 2 Mar 2021 11:18:11 -0800 Subject: [PATCH 4/5] Fix mistake when joining a path. --- src/extension.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension.ts b/src/extension.ts index c066860ef..6a428755a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -443,7 +443,7 @@ class ExtensionManager implements vscode.Disposable { const sourceDirectory: string = cmt.workspaceContext.config.sourceDirectory; let expandedSourceDirectory: string = util.lightNormalizePath(await expandString(sourceDirectory, { vars: optsVars })); if (path.basename(expandedSourceDirectory).toLocaleLowerCase() !== "cmakelists.txt") { - expandedSourceDirectory = path.join(expandedSourceDirectory, "/CMakeLists.txt"); + expandedSourceDirectory = path.join(expandedSourceDirectory, "CMakeLists.txt"); } if (await fs.exists(expandedSourceDirectory)) { if (should_configure) { From 0a13c2a9654ef96411fb44969939e420bfe12807 Mon Sep 17 00:00:00 2001 From: Andreea Isac Date: Tue, 2 Mar 2021 11:23:33 -0800 Subject: [PATCH 5/5] Fix bad merge. --- src/extension.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/extension.ts b/src/extension.ts index dfde1841c..9442b6e1b 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -37,7 +37,6 @@ import {ProjectOutlineProvider, TargetNode, SourceFileNode, WorkspaceFolderNode} import * as util from '@cmt/util'; import {ProgressHandle, DummyDisposable, reportProgress} from '@cmt/util'; import {DEFAULT_VARIANTS} from '@cmt/variant'; -import paths from './paths'; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle();