Skip to content

Conversation

@will-v-pi
Copy link
Collaborator

Instead of different DO NEVER EDIT lines for each SDK version, include a common file which does all that logic

Update any existing CMakeLists.txt files to this new style when extension is loaded, and also copy the pico-vscode.cmake into place whenever the extension is loaded to ensure it is up-to-date

This will make future updates which require changes to that section of the file much simpler, as the separate pico-vscode.cmake file can be updated instead

"endif()\n"
f"set(sdkVersion {params['sdkVersion']})\n"
f"set(toolchainVersion {params['toolchainVersion']})\n"
f"set(picotoolVersion {params['picotoolVersion']})\n"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this also directly fixes an issue I noticed during testing that currently the extension always generates a CMakeLists.txt with a picoTool version equal to it's selected SDK version (see L593).

Copy link
Collaborator

@paulober paulober left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great addition for the modularity and platform independence of pico projects.
I tested it with different projects and it worked without problems in all scenarios we implemented before like switching SDK version.

Instead of different DO NEVER EDIT lines for each SDK version, include a common file which does all that logic

Update any existing CMakeLists.txt files to this new style when extension is loaded
Use USERHOME, rather than a full path
@will-v-pi will-v-pi marked this pull request as ready for review August 23, 2024 15:05
@will-v-pi will-v-pi merged commit 060ccee into main Aug 23, 2024
@will-v-pi will-v-pi deleted the cmake-include branch August 23, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants