vscode-cmake-tools icon indicating copy to clipboard operation
vscode-cmake-tools copied to clipboard

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

Open danniesim opened this issue 1 year ago • 10 comments

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

  • Fixes #2338
  • Allow users to turn off CMake Tools for multi-root workspaces where not all roots contain CMake projects.
  • Soothe my soul, as my pet peeve is seeing a Python root appear under CTest because CMake Tools is enabled for all roots in a workspace.

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:

  • Fixes #3278
  • Fixes #3170 via a more intuitive "Disable for folder" paradigm. Also, unlike cmake.ignoreCMakeListsMissing, this prevents folders from appearing in under CTest in the Test Explorer.
  • Fixes #1069

danniesim avatar Mar 10 '24 21:03 danniesim

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

danniesim avatar Apr 09 '24 12:04 danniesim

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 avatar Apr 10 '24 00:04 gcampbell-msft

@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!

gcampbell-msft avatar Apr 11 '24 18:04 gcampbell-msft

@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.

danniesim avatar Apr 11 '24 19:04 danniesim

@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

gcampbell-msft avatar Apr 19 '24 20:04 gcampbell-msft

Yes, you nailed that one. There is an unpopulated CTest stub for every project in a workspace regardless if ignoreCMakeListsMIssing is true or false. e.g. Even if a root is a Python project, it will have a CTest stub. You can see this in action by adding a Python project to a workspace with CMake projects.

Also, it's unintuitive to set 3 settings to the right combination just to get one feature - which is simply to tell the CMake extension that a project root in a multi-root workspace is not a CMake project hence the extension should be disabled for that project root.

The key here is that in multi-root, multi-language workspaces, we want it to be easy to tell the extension which roots are CMake and which are not.

We are getting warmer with #3703. However, we want also to find a solution for configuring a multi-root, multi-language workspace and not just "on open".

danniesim avatar Apr 19 '24 22:04 danniesim

@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.

gcampbell-msft avatar Apr 22 '24 13:04 gcampbell-msft

@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

danniesim avatar Apr 24 '24 17:04 danniesim

@gcampbell-msft Is there any chance to get someone to look at and review this in order to progress it towards a merge?

I would really love to have this functionality available as currently I'm stuck with CMake Tools trying to configure a project in a multi root workspace which I'd like it not to touch at all.

blubberdiblub avatar Jul 23 '24 11:07 blubberdiblub

@blubberdiblub @danniesim Sorry for the delay on reviewing this. I've taken a note to discuss this PR with the team next week. In the meantime, @danniesim it would be helpful to fix the merge conflicts that are now present after some changes we've made. Thanks!

gcampbell-msft avatar Jul 26 '24 14:07 gcampbell-msft

The latest CI build (Window) gives a timeout error. https://github.com/microsoft/vscode-cmake-tools/actions/runs/11215290289/job/31172174084?pr=3646 It doesn't seem like a code/script error. Shall we try it again?

danniesim avatar Oct 08 '24 09:10 danniesim

Hi, @danniesim.

Unfortunately, we plan to close this PR and not merge it into the codebase. This is because we don't want to add a top-level setting into the extension that makes it too easy to disable the extension at a workspace, but especially at a user level.

However, we very much want to improve this scenario, so I've created another overarching issue here: #4112, that is a high-level issue referencing the same issues you've referenced: #2338, #1069, #3278, #3170 and describes the general scenario and the high-level way that we'd currently like to solve it.

While we can't make a guarantee of when we will be able to implement a design change to fix/improve this scenario, it's an important scenario to us and we hope to get to it as soon as we can.

Thanks for your contributions!

gcampbell-msft avatar Oct 08 '24 10:10 gcampbell-msft

@gcampbell-msft Sure, I'll publish my fork and extension for people who have to deal with multi-language workspaces. I need it right now, and I'm sure a number of people do, too. I will be on this for several years, so maintenance is not an issue.

Get the forked extension here

danniesim avatar Oct 08 '24 11:10 danniesim