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

Plugin doesn't handle cmake.sourceDirectory changes properly #4203

Open
Luke-zhang-mchp opened this issue Dec 6, 2024 · 8 comments
Open
Labels
bug a bug in the product

Comments

@Luke-zhang-mchp
Copy link

Luke-zhang-mchp commented Dec 6, 2024

Brief Issue Summary

I have a workspace structure that looks like this:

.
├── cmake-simple-demo
│   └── CMakeLists.txt
├── cmake-simple-demo_copy
│   └──  CMakeLists.txt
└──  cmake-simple-demo_copy_1
    └──  CMakeLists.txt

When VSCode is first opened, the CMake view is not listed under "Open View" and many CMake commands are missing. This is to be expected, since the default value of cmake.sourceDirectory is ${workspaceFolder}, but there is no CMakeLists file there. However, when I give the preference a value, the commands are still missing, and the CMake view is still not listed. They only show once I reload the IDE.

A related issue: when there is only 1 path in the cmake.sourceDirectory array, the path and name of the folder is automatically assumed to be the workspace folder, which is not always true.

Image

Related again: when there is an array of directories in the array, and one new entry is added, the path and name of the folder is also assumed to be the workspace folder. This is fixed on reload, and also does not occur if more than 1 entry is added at once.

Image

CMake Tools Diagnostics

Debug Log

Additional Information

I did some preliminary digging. The second issue is caused by this line: https://github.com/microsoft/vscode-cmake-tools/blob/main/src/projectController.ts#L279. It should instead always return this.sourceDir.

The last issue is caused by this line: https://github.com/microsoft/vscode-cmake-tools/blob/main/src/projectController.ts#L279. The parameter value only includes the newly added values, hence why checking for length > 1 is incorrect.

I wanted to make sure there weren't any edge cases I was missing making these changes. For example, CMakeProject.folderPath has a comment that says "For single-project folders, this is the WorkspaceFolder for historical reasons." I haven't looked at the first issue yet.

@github-project-automation github-project-automation bot moved this to Blocked in CMake Tools Dec 6, 2024
@github-actions github-actions bot added the triage label Dec 6, 2024
@Luke-zhang-mchp Luke-zhang-mchp changed the title CMake doesn't handle cmake.sourceDirectory changes properly Plugin doesn't handle cmake.sourceDirectory changes properly Dec 6, 2024
@Luke-zhang-mchp
Copy link
Author

Luke-zhang-mchp commented Dec 6, 2024

Update: here are the changes I figured were needed to fix these bugs. I'm still unsure about the changes to the folderPath method and related isMultiProjectWorkspace logic

Luke-zhang-mchp/vscode-cmake-tools@main...Luke-zhang-mchp:vscode-cmake-tools:config-change-fix

@Yingzi1234
Copy link
Collaborator

Yingzi1234 commented Dec 11, 2024

@Luke-zhang-mchp We would like to reproduce your problem, please provide a record of the reproduction steps in as much detail as possible. Thank you in advance!

@Yingzi1234 Yingzi1234 added more info needed More info is needed from the community for us to properly triage and investigate. and removed triage labels Dec 11, 2024
@Luke-zhang-04
Copy link

For the first issue:

  1. In an empty workspace, create a file in folder/CMakeLists.txt
  2. Open the workspace in VSCode, create a .vscode/settings.json file and set "cmake.sourceDirectory": "${workspaceFolder}/folder"
  3. In the command palette, you can see the CMake feature set is not enabled (most commands are missing). Reloading the IDE fixes this.

For the second issue, open command palette and run "Select Active Folder". The dropdown option should have title folder, but instead it is the name of the workspace. The path that is displayed next to it is ${workspaceFolder}, but it should be ${workspaceFolder}/folder.

@Yingzi1234
Copy link
Collaborator

@Luke-zhang-04 Thank you for your reply! Based on your description of the problem, we have reproduced the problem, but we are not sure if we have reproduced your problem completely, here is a recording of me reproducing the problem, could you help me to confirm if we have reproduced the problem?
Image

@Luke-zhang-mchp
Copy link
Author

Luke-zhang-mchp commented Dec 18, 2024

Hey @Yingzi1234, that doesn't look like what I was trying to get at. Here's the first problem:

recording.issue.1.mp4

as you can see, the CMake view and CMake commands only show up after IDE refresh.

The second problem:

recording.issue.2.mp4

In the select active folder command, the names of the project are sometimes wrong. This is the case when either:

  • There is only 1 valid project in the cmake.sourceDirectory array
  • 1 valid project was added to the cmake.sourceDirectory array, in which case the new project is the one with the incorrect name.

Like I said, these were the changes I needed to fix the issue: Luke-zhang-mchp/vscode-cmake-tools@main...Luke-zhang-mchp:vscode-cmake-tools:config-change-fix, but I'm not sure if my changes will break something else or what the "historical reasons" mentioned in the comments are.

@Yingzi1234
Copy link
Collaborator

@Luke-zhang-04 Thank you for your reply and help! With your help, we reproduced both of your issues and we have turned this issue into a bug
@gcampbell-msft The user has a total of two issues that we can reproduce, here are the detailed reproduction steps as well as the information, the user has already provided the fix for the PR on his own but he doesn't know how to do the merge, could you provide some help?

The first issue: When CMake. source directory specifies only one subfolder, the list after running CMake: Select Active folder will not show the expected results.
Repro steps:

  1. Open this folderMyWorkspace.zip(Make sure the test folders have two or more sub-folders)
  2. Set “CMake.source directory” to one of the sub-project names.
  3. Click F1 to run the command CMake: select active folder and check that the entire list is displayed
    Image

Second issue: When an invalid subfolder exists in the CMake.source directory, the list is incomplete after running the CMake: Select Active command
Repro steps:

  1. Open this folderInheritance.zip (Make sure the test folders have two or more sub-folders)
  2. Set the sub-folder's name to the CMake. source directory settings
  3. Invalidate one of the lines of code
  4. Click F1 to run the command CMake: select active folder and check that the entire list is displayed
    Image

@Yingzi1234 Yingzi1234 added bug a bug in the product and removed more info needed More info is needed from the community for us to properly triage and investigate. labels Dec 19, 2024
@Luke-zhang-mchp
Copy link
Author

Just a note: that second issue described in the video recording is actually the opposite of what I had initially intended to fix. For us, we started with cmake.sourceDirectory as an empty array, and then when adding projects to it, the CMake feature set was not activated (i.e could not open CMake view, could not run most CMake commands). However the code needed to fix both issues is the same.

@Yingzi1234
Copy link
Collaborator

@Luke-zhang-04 Thanks for the additions, we've changed the status of this issue to Bug, we'll let you know as soon as we have any updates!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a bug in the product
Projects
Status: Blocked
Development

No branches or pull requests

3 participants