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.

Sign up for GitHub

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

Jump to bottom

Improve debug configuration providers #256

Merged
merged 4 commits into from
Aug 16, 2024
Merged

Improve debug configuration providers #256

merged 4 commits into from
Aug 16, 2024

Conversation

ylatuya
Copy link
Contributor

@ylatuya ylatuya commented Aug 8, 2024

The following improves the debug configuration providers:

  • Fix a bug in the creation of debug configurations for cppdbg and cppvsdbg
  • Register cppdbg and cppvsdbg providers separetly
  • Only register the providers if there is a debugger extension installed.

ylatuya added 2 commits August 8, 2024 09:32
@ylatuya
debug: fix list of debug configurations for C++ (GDB/LLDB) debugger
2aea915 
createDebugConfiguration is silently failing returning an empty
list of debug configurations.
@ylatuya
debug: fix DebugConfigurationProvider registration
65bfd5b 
We are incorrectly registering a single debug provider that provides
`cppdbg` and `cppvsdbg` configurations using the `cppdbg` type.

Split the cppdb and cppvsdbg debug providers into 2 different providers
and register them based on the current operating system following the
same approach as the `ms-vscode.cpptools` extension.

Also simplify the debug providers into a single MesonDebugProvider
class.
@ylatuya ylatuya force-pushed the fix-debug-configuration branch from 5184bc5 to fb4d6a8 Compare August 8, 2024 10:45
@ylatuya
Copy link
Contributor Author

ylatuya commented Aug 8, 2024 •
edited
Loading

Here is an example of providing debug configuration for the lldb debugger type when the extension is not installed:
Screenshot 2024-08-07 at 14 22 54

@ylatuya ylatuya force-pushed the fix-debug-configuration branch from 4e34352 to fb4d6a8 Compare August 8, 2024 10:52
@ylatuya
Copy link
Contributor Author

ylatuya commented Aug 8, 2024

I am not sure how to handle the case of a missing debugger extension:

  1. Only register the provider if the extension exists
  2. Always register the providers, but only provide debug configurations when the extension exists.

@cskeogh cskeogh mentioned this pull request Aug 9, 2024
Closed
tristan957
tristan957 reviewed Aug 9, 2024
View reviewed changes
src/debug/index.ts Outdated Show resolved Hide resolved
@tristan957
Copy link
Contributor

What line does the undefined come from?

@ylatuya
Copy link
Contributor Author

ylatuya commented Aug 13, 2024

What line does the undefined come from?

undefined comes from registering a debug configuration provider using vscode.debug.registerDebugConfigurationProvider with a debugger type that is unknown, for example registering it for the lldb debugger type when this debugger does not exists.

Debuggers are registered by extensions like CodeLLDB here registering a new debugger with type: lldb and label: LLDB.

@tristan957
Copy link
Contributor

undefined comes from registering a debug configuration provider using vscode.debug.registerDebugConfigurationProvider with a debugger type that is unknown, for example registering it for the lldb debugger type when this debugger does not exists.

Does this mean that we should check for the existence of the debugger before registering the provider?

@ylatuya
Copy link
Contributor Author

ylatuya commented Aug 13, 2024 •
edited
Loading

undefined comes from registering a debug configuration provider using vscode.debug.registerDebugConfigurationProvider with a debugger type that is unknown, for example registering it for the lldb debugger type when this debugger does not exists.

Does this mean that we should check for the existence of the debugger before registering the provider?

This is what this PR does here: fb4d6a8

My doubt is whether to do that check when registering the provider or when listing configurations in MesonDebugConfigurationProvider.provideDebugConfigurations for the corner case in which a user has the meson extension installed and initialized and they install the extension providing the debugger support afterwards, which would require restarting vscode. Supporting that corner case would make the code less obvious, and I am unsure about the real benefits.

ylatuya added 2 commits August 13, 2024 17:08
@ylatuya
debug: register providers only if the debugger extension is installed
a756e9b 
Providing debug configurations if the debugger extension is not
installed has 2 problems:
  * The debug configuration is listed in a menu item named "undefined"
  * The launch configuration will not work because the debugger does not
    exists
@ylatuya
debug: use lldb or gdb for cppdbg using the OS defaults
37278d5 
We shouldn't infer what debugger to use based on the compiler
command. Instead we use the defaults based on the OS:
https://code.visualstudio.com/docs/cpp/launch-json-reference#_mimode
@ylatuya ylatuya force-pushed the fix-debug-configuration branch from 641eaba to 37278d5 Compare August 13, 2024 15:09
@tristan957
Copy link
Contributor

I don't think we need to solve the corner case. I like what you have. Are you ready for me to merge it?

@ylatuya
Copy link
Contributor Author

ylatuya commented Aug 13, 2024

I don't think we need to solve the corner case. I like what you have. Are you ready for me to merge it?

It's ready to be merged :) Thanks for the review

@tristan957 tristan957 added this pull request to the merge queue Aug 16, 2024
Merged via the queue into mesonbuild:main with commit 5cc1b7b Aug 16, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tristan957 tristan957 tristan957 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ylatuya @tristan957