-
Notifications
You must be signed in to change notification settings - Fork 464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix1914 - Variable substitution stopped working with release 1.7 #3268
Fix1914 - Variable substitution stopped working with release 1.7 #3268
Conversation
…ated to sourceDirectory and kit info
…ttps://github.com/microsoft/vscode-cmake-tools into dev/andris/cmake_tools/fix1914_varexp_kit_sourceDir
src/presetsController.ts
Outdated
@@ -42,12 +43,26 @@ export class PresetsController { | |||
const presetsController = new PresetsController(project, kitsController, isMultiProject); | |||
const expandSourceDir = async (dir: string) => { | |||
const workspaceFolder = project.workspaceFolder.uri.fsPath; | |||
const kit = project.getActiveKit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part worries me, if we're in the presetsController then don't we not care about kits? Since it's either kits or Presets? Not both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't step back and realize this obvious fact. I just noticed errors and complains in my repro and just went head first into making those go away. I'll rethink a bit and get back with an update. Very good essential point.
src/drivers/cmakeDriver.ts
Outdated
const ws_root = util.lightNormalizePath(workspaceFolderFspath || '.'); | ||
|
||
// Fill in default replacements | ||
if (!kit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same worry here, I'm not sure we want to be using kits every time the cmakeDriver uses this method, since couldn't a preset use this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect point, I'll rethink and be back with an update.
The last PR update does the following:
|
Fix1914 - Variable substitution stopped working with release 1.7, related to sourceDirectory and kit info variables.
#1914
Will add explanations later, do more cleanup and also ensure more variables are considered. For now, just kit name and vendor, two examples that are different and exist in different data structures. Creating the PR to see tests outcome.