Skip to content
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

Configure only one project for CMakelists.txt node in ProjectOutline #3372

Merged
merged 8 commits into from
Oct 13, 2023

Conversation

vlavati
Copy link
Contributor

@vlavati vlavati commented Oct 10, 2023

This change addresses item #3311

This changes visible behavior

The following changes are proposed:

  • configure only project, that corresponds to CMakeLists.txt file node, instead of all projects

The purpose of this change

The issue #3311 describes case with few projects in the ProjectOutline and it is possible to configure all projects, but there is no way to configure only a project that corresponds to the selected CMakeLists.txt. This PR changes the behavior of "configure" button to configure only one.

@gcampbell-msft gcampbell-msft self-assigned this Oct 10, 2023
Copy link
Collaborator

@gcampbell-msft gcampbell-msft left a comment

Choose a reason for hiding this comment

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

Everything looks really good except for my single comment. Thanks!

src/extension.ts Outdated
@@ -1876,7 +1876,13 @@ async function setup(context: vscode.ExtensionContext, progress?: ProgressHandle
vscode.commands.registerCommand('cmake.outline.editCacheUI', () => runCommand('editCacheUI')),
vscode.commands.registerCommand('cmake.outline.cleanRebuildAll', () => runCommand('cleanRebuildAll')),
// Commands for outline items
vscode.commands.registerCommand('cmake.outline.configure', (what: ProjectNode) => runCommand('configure', what.folder, false, what.sourceDirectory)),
vscode.commands.registerCommand('cmake.outline.configure', async (what: object) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It'd be good if we could restrict this type more than just object, something that only allows for ProjectNode or SourceFileNode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review.
I've changed type annotation. Is it better?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes! Everything looks good. Please add a Changelog entry and then I will approve and merge!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@gcampbell-msft gcampbell-msft enabled auto-merge (squash) October 13, 2023 09:53
@gcampbell-msft gcampbell-msft merged commit 2697ab3 into microsoft:main Oct 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants