Add automatic kit scanning functionality#351
Add automatic kit scanning functionality#351drook207 wants to merge 33 commits intoCivitasv:masterfrom
Conversation
lceWolf
left a comment
There was a problem hiding this comment.
I like the idea, it replicates vscode's automatic kit detection 👍
Tilde was not exapanded correctly, that results in creating the kits file in creating the whole path in $PWD
- Add a test for kit scanning - Add a test that checks if the file is present and if the kits could be registered
Backmerge Main
- Complete reimplementation of the scanner functionality using nvim's internal functions and a more generic approach. - Add possibility to detect cross platform compiler - Make the scanner more extensible - Add unit tests for the new functions
|
Hi guys, sorry that it took so long to continue, but as I got back to it i decided to start right from the beginning. I now use the nvim api for all the detection functions and I try to make it as extensible as possible. For now we are able to detect clang and gcc compiler on linux systems (And cross compilation compilers as well). I would suggest to do another PR for the WIndows MSVC and MinGW functionality. |
|
And one further question. Should we regenerate the cmake kits file entirely when the user command is triggered or should we append only new kits to the file? |
| -- if exists cmake-kits.json, kit is used to set | ||
| -- environmental variables and args. | ||
| local kits_config = kits.parse(const.cmake_kits_path, config.cwd) | ||
| local kits_config = kits.parse(const.cmake_config_path, config.cwd) |
There was a problem hiding this comment.
I think this has to stay const.cmake_kits_path, right?
If so, do we need const.cmake_config_path at all?
There was a problem hiding this comment.
The parse function expects not only a path but also the filename for the cmake kits. But I agree to leave the const.cmake_kits_path nil.
The questions is should we add the kits filename to the const table and concat the config path and the filename ?
| cmake.register_autocmd() | ||
| cmake.register_autocmd_provided_by_users() | ||
| cmake.register_scratch_buffer(config.executor.name, config.runner.name) | ||
| if not vim.uv.fs_stat(const.cmake_kits_path) then |
There was a problem hiding this comment.
maye we should add an option like we did for the presets to enable kit usage?
I think it might be worth keeping const.cmake_kits_path = nil as default and not change existing behaviour 🤔
| cmake.register_autocmd_provided_by_users() | ||
| cmake.register_scratch_buffer(config.executor.name, config.runner.name) | ||
| if not vim.uv.fs_stat(const.cmake_kits_path) then | ||
| scanner.scan_for_kits() |
There was a problem hiding this comment.
this is a synchronous which can block the ui on startup. I think it is work running async, right?
| end | ||
|
|
||
| local function get_path_executables() | ||
| local path_dirs = vim.split(vim.env.PATH or "", ":", { plain = true }) |
There was a problem hiding this comment.
this will break on windows
| local seen = {} | ||
| local chains = {} | ||
|
|
||
| for exe in pairs(executables) do |
There was a problem hiding this comment.
I think this filtering could be moved to the point of searching the paths variable
There was a problem hiding this comment.
I don't get what you mean, sorry. Could you maybe give a further explenation?
| return { | ||
| c = (prefix or "") .. c_name, | ||
| cxx = (prefix or "") .. companions.cxx, | ||
| linker = (prefix or "") .. companions.linker, |
There was a problem hiding this comment.
I think this is actually wrong for lld. See It is always a cross-linker, meaning that it always supports all the above targets however it was built. In fact, we don’t provide a build-time option to enable/disable each target. This should make it easy to use our linker as part of a cross-compile toolchain.
There was a problem hiding this comment.
Ok thanks again for the docs. So should we maybe take out the linker detection complety? Becaus if I get it correct, vscode's cmake-tools does not add the linker at all. It is again a manual step.
| kit.compilers.C = get_executable_path(tc.c) | ||
|
|
||
| if has_cxx then | ||
| kit.compilers.CXX = get_executable_path(tc.cxx) |
There was a problem hiding this comment.
I think this can be called unconditionally. vim.fn.exepath should yield the correct result
No worries. Thats quite the topic to tackle and you are moving in a good direction! :)
Maybe I missed a detail, but I think the PR might break existing setups on windows since the PATH parsing does only work for Unix systems |
How does the VS Code extension handle it? |
Refactor: Change cmake kits path back to nil
fix: remove duplicated code line
No they add and update only. I'm with you, stale kits should be cleaned if one triggers scan for kit manually |
Yes you were right, did not see that. I fixed the path separator so the plugin should continue to work on windows. But I will ensure that tomorrow with my work PO |
Summary
This PR adds automatic CMake kit detection and scanning capabilities to
cmake-tools.nvim, making it easier to get started without manually
configuring compiler kits.
~/.config/cmake-tools/cmake-kits.jsonCMakeScanForKitsuser command for manual scanningChanges
New module:
lua/cmake-tools/scanner.lua- Core kit scanningimplementation with:
Enhanced workflow: Automatic kit scanning when
cmake-kits.jsondoesn't exist
User command:
CMakeScanForKitsfor manual kit discoveryDefault config: Set
cmake_kits_pathto~/.config/cmake-tools/bydefault
Test plan
CMakeScanForKitscommand