-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
fix(dap): lldb library paths are strings #74
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: We'll have to make this dependent on whether we're using lldb
(adapter.type == 'executable'
) or codelldb
(adapter.type == 'server'
), otherwise it'll error with codelldb
(see this review comment).
I would suggest to convert the map into a list of strings if (adapter.type == 'executable'
).
PS, do you have a project I can test this on (ideally with steps to reproduce, for when I implement an integration test)? |
Yeah, good idea: I'll see what the smallest possible example with dynamic linking is, no sense in throwing all of Bevy at it. |
Shoot, sry I missed that one in all the confusion 🙂 |
https://github.com/richchurcher/test-rustaceanvim reproduces the problem. I forgot that there's a dynamic linking feature in Bevy specifically, so did need to include it. Let me know if you want me to leave it there indefinitely or remove it. |
Thanks 🙏 I'll have to pick this up on Friday, but if you're okay with it, we can add it to this repo under a |
lldb-dap needs a list of strings, codelldb needs a map. Fixes mrcjkb#73.
38db374
to
b376079
Compare
💯 The latest commit seems to do the trick. Managing a |
Just tested it with
The
to enter a shell with the adapter from my fork. |
Description of changes
This:
lldb
is fussy about this apparentlyI can't promise this is also true for code-lldb? But it seems likely. Tests are still green.
Fixes #73
Things done
busted
specs