Closed spindensity closed 3 years ago
I've updated the pull request according to the suggestions. I did not change the example since I hope the CodeLLDB
example work out-of-the-box, the args
supported by the hard-coded configuration has a nested table structure that is incompatible with CodeLLDB
(CodeLLDB
only supports a string list as args
), and original cwd
is also incompatible with CodeLLDB
. They should be overridden.
the args supported by the hard-coded configuration has a nested table structure that is incompatible with CodeLLDB(CodeLLDB only supports a string list as args), and original cwd is also incompatible with CodeLLDB.
Let's check for type
and adapt args
and cwd
that passed by the extension to make it compatible if with CodeLLDB
. Otherwise you won't be able to define arguments from the extension.
the args supported by the hard-coded configuration has a nested table structure that is incompatible with CodeLLDB(CodeLLDB only supports a string list as args), and original cwd is also incompatible with CodeLLDB.
Let's check for
type
and adaptargs
andcwd
that passed by the extension to make it compatible if withCodeLLDB
. Otherwise you won't be able to define arguments from the extension.
But the value of the type
can be any non-empty string.
But the value of the type can be any non-empty string.
Sure, I suggesting to apply the required adaptations only if the string is equal to codelldb
. If I understand correctly, we should edit args
and cwd
a little, right? It will be much better then edit Neovim configuration each time you want to change args for debugging.
But the value of the type can be any non-empty string.
Sure, I suggesting to apply the required adaptations only if the string is equal to
codelldb
. If I understand correctly, we should editargs
andcwd
a little, right? It will be much better then edit Neovim configuration each time you want to change args for debugging.
Could you tell me why args
passed to the program has a data structure like this, is it by design?
I think the args
passed to the program to be launched should be a list of string, and the code parsing args
should be like this, but I am not sure whether you have some special considerations with a nested table structure as my previous post pointed out:
local splitted_args
if arguments then
splitted_args = vim.fn.split(arguments, [[\s\%(\%([^'"]*\(['"]\)[^'"]*\1\)*[^'"]*$\)\@=]])
else
splitted_args = {}
end
-- Remove quotes
for i, arg in ipairs(splitted_args) do
splitted_args[i] = arg:gsub('"', ''):gsub("'", '')
end
for _, arg in ipairs({ ... }) do
table.insert(splitted_args, arg)
end
Then we do not need to override the args
with user settings and all the problems related should be gone.
I think the args passed to the program to be launched should be a list of string, and the code parsing args
Yes, and I think this is how it passed now. Am I wrong?
I think the args passed to the program to be launched should be a list of string, and the code parsing args
Yes, and I think this is how it passed now. Am I wrong?
You passed a nested table to the program to be launched:
Oh, I get it, I guess it happens by mistake when there are no arguments. Maybe vscode-lldb
just handles this case somehow. Let's fix it as you suggested by passing an array of strings.
How about something like this?
local splitted_args
if not arguments then
splitted_args = {}
else
splitted_args = vim.fn.split(arguments, [[\s\%(\%([^'"]*\(['"]\)[^'"]*\1\)*[^'"]*$\)\@=]])
-- Remove quotes
for i, arg in ipairs(splitted_args) do
splitted_args[i] = arg:gsub('"', ''):gsub("'", '')
end
end
vim.list_extend(splitted_args, { ... })
Yes, the same logic as the code I posted in https://github.com/Shatur/neovim-cmake/pull/7#issuecomment-931244632, if you prefer to vim.list_extend
, that's OK.
Yes, the same logic as the code I posted in #7 (comment), if you prefer to vim.list_extend, that's OK.
Yes, I just modified it a little :) Does it works for CodeLLDB?
Yes, the same logic as the code I posted in #7 (comment), if you prefer to vim.list_extend, that's OK.
Yes, I just modified it a little :) Does it works for CodeLLDB?
After the args parsing bug is fixed, we do not need to override args
and cwd
, I will update the CodeLLDB
example according to the new configurations posted on nvim-dap wiki and https://github.com/mfussenegger/nvim-dap/issues/307#issuecomment-931198952 just a few hours ago.
Awesome!
Another question:
In https://github.com/Shatur/neovim-cmake/pull/7#discussion_r719225453, you said "Let's move local config definitions here to avoid creating and extending the table every time:", but args
and program
are dynamically created, we could not avoid creating the table every time, if we provide some default key-value pairs, it means we need to extend and override the table every time even if we do not customize this variable in user settings. I still prefer to using the empty table as the default value of g:cmake_dap_configuration
.
Any idea?
Done.
but args and program are dynamically created, we could not avoid creating the table every time, if we provide some default key-value pairs, it means we need to extend and override the table every time even if we do not customize this variable in user settings
This is true, but the user will see the default values more explicitly in this case. So I thinking that extending the configuration table with args
and cwd
should be okay.
but args and program are dynamically created, we could not avoid creating the table every time, if we provide some default key-value pairs, it means we need to extend and override the table every time even if we do not customize this variable in user settings
This is true, but the user will see the default values more explicitly in this case. So I thinking that extending the configuration table with
args
andcwd
should be okay.
The problem is we must ensure these keys exist. So the suggested change in https://github.com/Shatur/neovim-cmake/pull/7#discussion_r719225453 would not work if users customize the settings as follows:
vim.g.cmake_dap_configuration = {
stopOnEntry = false,
runInTerminal = false,
}
Users must copy all required keys if they want their customization to work.
Do you think we should handle that with something like this:
if exists('g:cmake_dap_configuration')
let g:cmake_dap_configuration = extend({'type': 'cpp', 'name': 'Debug CMake target', 'request': 'launch'}, g:cmake_dap_configuration)
else
let g:cmake_dap_configuration = {'type': 'cpp', 'name': 'Debug CMake target', 'request': 'launch'}
endif
or just let users make it right themselves?
The problem is we must ensure these keys exist. So the suggested change in #7 (comment) would not work if users customize the settings as follows:
Hm... This actually sounds pretty reasonable. But for now, I would "let users make it right themselves" just for consistency, because other things are done in the same way.
In future I would switch to setup()
approach as most plugins do and handle mentioned stuff on plugin side by extending the table for all options.
One more suggestion: let's generate name
from target name instead of using Debug CMake target
. This will allow the user to avoid specifying an unnecessary name from the configuration and will look more informative for the debugger or for displaying in the statusline.
Done.
Done.
Fix #6.