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

Update CMake Tools experience on open #3703

Closed
wants to merge 8 commits into from

Conversation

gcampbell-msft
Copy link
Collaborator

@gcampbell-msft gcampbell-msft commented Apr 11, 2024

Addresses #3588.

The goal of this PR is to make it easier and more obvious for people to utilize the cmake.ignoreCMakeListsMissing setting. This setting allows people to turn off the searching for a CMakeLists.txt quick pick when they are opening a project that does contain a CMakeLists.txt somewhere, but they aren't intending on using the folder as a CMake project.

This changes the default for cmake.configureOnOpen to true. It also removes the "Would you like to configure" pop-ups.

For the pop-up to select the CMakeLists.txt, we only pop this up on startup when cmake.configureOnOpen is true if we actually find a CMakeLists.txt within the folders that would've activated the extension.

@vlavati FYI, this is making a slight modification from your PR here: #3276

snehara99
snehara99 previously approved these changes Apr 11, 2024
@gcampbell-msft gcampbell-msft requested review from moyo1997 and snehara99 and removed request for snehara99 April 11, 2024 19:46
@gcampbell-msft gcampbell-msft marked this pull request as draft April 15, 2024 16:07
@vlavati
Copy link
Contributor

vlavati commented Apr 16, 2024

@gcampbell-msft , thanks for the note!

I tried #3588 on current main branch and the dialog to select CMakelists is not shown only in case if "configureOnOpen" is persisted to "false". Then to configure the project it is possible to choose CMakelists.txt in vscode Explorer tree and run configure from context menu, or it is possible to run configure from the command palette (Ctrl+Shift+P).

In case of PR, if user opens folder and select "Not now" on "Would you like to configure project?", it is possible to see:
image

  • on top - the dialog to select files with new option
  • and on bottom right - the dialog to persist option "configureOnOpen"

In case of response "Not now", it looks confusing to see dialog to select CMakelists, but in case if option "configureOnOpen" is already persisted with "false", it is probably strange to see nothing if folder is opened.
May be it is possible to implement steps like this:

  • on the folder is opened the extension tries to find projects (the configured projects in the settings or the project(CMakeLists.txt) in the root of folder)
  • if nothing is found,
  • then dialog to select CMakeLists is shown if no option "cmake.ignoreCMakeListsMissing", the dialog should be with option to "ignoreCMakeListsMissing" like it is implemented in current PR, the continuation of dialog should not check option "configureOnOpen", because right now the usecase is transformed to the setup of the project.
  • else the option "configureOnOpen" is checked.

So the option "configureOnOpen" is checked only in case of existing and previously configured projects. And if user opens the folder for the first time, the dialog to select CMakeLists is shown.

I hope this workflow is possible to implement and it will not break any user experience.

@gcampbell-msft gcampbell-msft changed the title allow for don't show again option for CMakeLists.txt quick pick Update CMake Tools experience on open Apr 19, 2024
@gcampbell-msft gcampbell-msft marked this pull request as ready for review April 19, 2024 20:55
@danniesim
Copy link
Contributor

@gcampbell-msft At this point, we might want to reduce the scope of #3646 to: Remove CTest project stubs of projects with ignoreCMakeLists set to true. That'll effectively make the folder scoped ignoreCMakeLists be the setting to enable/disable CMake Tools for a folder.

@gcampbell-msft
Copy link
Collaborator Author

@gcampbell-msft At this point, we might want to reduce the scope of #3646 to: Remove CTest project stubs of projects with ignoreCMakeLists set to true. That'll effectively make the folder scoped ignoreCMakeLists be the setting to enable/disable CMake Tools for a folder.

I agree. Also, it feels to me like the main issue is the ctest integration, and so it also feels to me like we should simply do a better job not integrating with the test explorer if there isn't a "valid" CmakeProject to interact with. In this case, a "valid" CMakeProject would refer to one that has a CMakeLists.txt defined.

@gcampbell-msft gcampbell-msft deleted the dev/gcampbell/AskForCmakeListsUpdate branch October 14, 2024 12:36
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.

7 participants