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

FOLDER grouping in default build target dropdown #3953

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

itzandroidtab
Copy link

This changes visible behavior

The following changes are proposed:

  • Grouping default build targets dropdown based on CMake FOLDER property

The purpose of this change

When a project has a lot of targets it becomes a hassle to find the right default build target. In the project outline the targets are grouped. This PR also groups the default build target dropdown. Discussion #3949. When clicking on the folder it will show the corresponding targets.

Previous

image

New

image
After selecting the folder:
image

@itzandroidtab
Copy link
Author

@gcampbell-msft @qarni What would I need to do to have this PR merged?

@gcampbell-msft
Copy link
Collaborator

@itzandroidtab Our apologies for the delay, we've been in the midst of the 1.19 official version release, so we've not been able to review community PR's. We hope to get to this soon, thanks and we appreciate your patience.

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.

@itzandroidtab First, please include a CHANGELOG.md entry.

Also, I like the idea of this PR and the screenshots you've provided look really good! I haven't taken a deeply close look at the code, but again it looks good.

HOWEVER, I think that this is probably a feature change that each user should be able to control if they would like. I think it'd be a great idea to add a setting that would allow people to choose whether they want this to be a flat list, or whether they want it to be structured using the FOLDER property. I could imagine a scenario where they only have a couple of targets, but if they're using the FOLDER property, even though there are only a limited set of options, they have to do multiple clicks to select it, due to the new implementation you've added.

I'd love to approve this feature, but I think it should have the added ability to control whether we use the flat list or the FOLDER grouping based on a setting. I currently don't have a strong opinion on whether the setting should default to the flat list or the FOLDER grouping.

@moyo1997 @itodirel @sinemakinci1 FYI, thoughts?

@itzandroidtab
Copy link
Author

@gcampbell-msft What would you call this option? And should this option also apply to the side view?

@gcampbell-msft
Copy link
Collaborator

@gcampbell-msft What would you call this option? And should this option also apply to the side view?

I don't believe this option should apply to the side view, the FOLDER grouping should be used there always. The setting should only be used in the build target dropdown.

For the name, it could be something like:
cmake.useFolderPropertyInBuildTargetDropdown

@itzandroidtab
Copy link
Author

Thank you. I will work on this

@gcampbell-msft gcampbell-msft marked this pull request as draft December 9, 2024 19:18
@gcampbell-msft
Copy link
Collaborator

gcampbell-msft commented Dec 9, 2024

@itzandroidtab Changing this to a draft PR until further changes are made. Please let us know if you have any questions but we will leave it in a draft state for a while, and then if no updates are made we may need to close it in order to keep our PRs clean. We're interested in considering this change once the settings option is added! Thanks!

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