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

Enable/disable CMake Tools at folder, workspace or user level #3646

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

danniesim
Copy link
Contributor

@danniesim danniesim commented Mar 10, 2024

This change addresses items #2338, #3278, #3170 and #1069

The following changes are proposed:

Add a setting that allows users to disable CMake Tools at the folder (aka resource) scope.

The purpose of this change

Other Notes/Information

I am dogfooding this change with my multi-root workspace with Python, Fortran, Node.JS and C++ CMake projects.

So far, so good.

Edit 1:

  • Fix: Hide the CMake Tools Sidebar Button when no CMake folders are enabled.
  • Fix: Select the first enabled folder if the active folder is disabled.
  • Fix: Dispose the subscription to the enabled setting before adding another.

Edit 2:
I have tested it in multi-root Node.JS, Python, C++ and Fortran projects, and it's working great.

Edit 3:

@danniesim danniesim force-pushed the dev/dannie.sim/FolderScopeEnable branch from 76e2486 to 27d2e99 Compare March 11, 2024 01:41
@danniesim danniesim marked this pull request as ready for review March 11, 2024 08:59
@danniesim danniesim force-pushed the dev/dannie.sim/FolderScopeEnable branch from 27d2e99 to 1a366e1 Compare March 16, 2024 10:09
@danniesim danniesim changed the title Folder scope setting to disable CMake Tools Folder scope setting to enable/disable CMake Tools Mar 16, 2024
@danniesim danniesim force-pushed the dev/dannie.sim/FolderScopeEnable branch from 1a366e1 to c242021 Compare March 16, 2024 13:31
@danniesim danniesim changed the title Folder scope setting to enable/disable CMake Tools Enable/disable CMake Tools at folder, workspace or user level Mar 16, 2024
@danniesim
Copy link
Contributor Author

@gcampbell-msft Would love some feedback on this. :) I've been using this branch and it works great for me.

@gcampbell-msft
Copy link
Collaborator

Hi @danniesim, before reviewing this PR, we'd like to understand more about these scenarios and why setting the cmake.configureOnOpen and other types of settings don't already handle this.

To my knowledge, the popup should appear for whether the cmake tools extension should configure, and a user can simply say "no" to this, and then answer the follow-up prompt for when it should honor that setting, and then doesn't that stop the cmake tools extension from trying to configure?

I'd love to discuss this and understand more about this. Thanks!

@gcampbell-msft
Copy link
Collaborator

@danniesim Curious to have your feedback from the above message.

Also, I'm curious about your thoughts on whether this PR #3703 fully solves this issue, or only partially.

My initial thoughts are that we already have settings that can be used to determine this, and my initial reaction to a setting that fully disables the extension is that I worry it could get set in the user level, and then disable CMake capabilities on Cmake projects by accident.

Instead, I think it might be better to instead use the PR I referenced above, and if that doesn't solve it all, we could make slight modifications that check if there is any CMake Project that has a CMakeLists.txt defined. If there are none, do like your PR suggestsions, but instead of using the setting to determine it, use the existence of a defined CMakeLists.txt.

More than willing to discuss this or have iterations, but looking forward to your feedback. Thanks!

@danniesim
Copy link
Contributor Author

@gcampbell-msft The Orta.vscode-jest extension does a similar disable for folder. I see your point: we do not want users to accidentally disable the extension at user scope. I presume VSCode does not allow a setting to exist only at folder scope?

#3703 does not help with non-Cmake projects appearing in the CTests explorer. That said, we can make this.addTestExplorerRoot(project.sourceDir); in ctests.ts run only when cmake.ignoreCMakeListsMissing is false. I am not sure if it will break anything as from what I've seen, the CTest code assumes that all roots are added and that all of them are CMake projects.

@gcampbell-msft
Copy link
Collaborator

@danniesim I'm revisiting this, and I'm trying to wrap my head around all the cases that you're seeing this happen.

From what I can tell, if users set cmake.configureOnOpen to false, and cmake.ignoreCMakeListsMissing to true, and cmake.ctest.testExplorerIntegrationEnabled to false, wouldn't this cover all of your cases?

I do notice that cmake.ctest.testExplorerIntegrationEnabled is at the user level, so my above assumption may not be true, but I'm just trying to understand what about the current support doesn't work for this, so that I can think through a way to get the outcome you want, without a possibly user level setting that disables the extension, if possible.

Also, notice the changes I've made to #3703

@danniesim
Copy link
Contributor Author

danniesim commented Apr 19, 2024 via email

@gcampbell-msft
Copy link
Collaborator

@danniesim Could you possibly send me a repro project / workspace to test with? I'd love to test this and while I could create miy own, it'd be good to ensure we have the same project setup. I'd like to investigate with, rather than using a setting to solve the ctest stub issue, simply determine based on whether we have a valid CMake Project (whether or not we actually have a CMakeLists.txt that is set up for the extension for that folder) to determine whether we should integrate ctest and the test explorer. Thanks.

@danniesim
Copy link
Contributor Author

@gcampbell-msft I have pushed a commit into this branch with a new multi-language workspace in the test folder.

Below is a gif I made to illustrate the UX issues we are discussing.

Multi-root and multi-language with CMake Tools Demo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants