Hide underlying debug adapter from the user #1024
Conversation
… correct debug adapter
matthewbastien
requested review from
adam-fowler,
0xTim and
award999
as code owners
|
Does this still give the user the option to use CodeLLDB when running Swift 6. |
Yup! I renamed the old setting that governed this to
Thanks for reminding me: I'll create a new issue to add tests for the debug adapter. A simple UI test to ensure that the debugger launches and hits breakpoints for each supported swift version should suffice (I'm not sure there's a good integration test for this and anything more than a simple UI test would be overkill and brittle in my experience). I'd also like to add unit tests for the creation of the launch config, but mocking the WorkspaceContext/FolderContext is too difficult at the moment. There'll probably need to be some changes to how the code is structured (potentially by adding DI) to make this easier. The overall testing plan for the extension right now is to add builds for macOS and Windows to cover all platforms. |
https://www.npmjs.com/package/@vscode/debugadapter-testsupport may be an option if we connected on a port instead of starting it up https://github.com/microsoft/vscode-debugadapter-node/blob/main/testSupport/src/debugClient.ts#L88. IDK how feasible though |
src/debugger/debugAdapter.ts
Outdated
| return swiftVersion.isLessThan(new Version(6, 0, 0)) ? "lldb-vscode" : "lldb-dap"; | ||
| public static getDebugAdapterType(swiftVersion: Version): "lldb-vscode" | "lldb-dap" { | ||
| return configuration.debugger.useCodeLLDB | ||
| ? "lldb-vscode" |
There was a problem hiding this comment.
question: does CodeLLDB even allow specifying a debug adapter?
There was a problem hiding this comment.
suggestion: assuming it is to make other code work below then maybe we should just adjust that logic
There was a problem hiding this comment.
No, CodeLLDB does not allow specifying a debug adapter. This is used in other places to determine which debug adapter we're using.
package.json
Outdated
| @@ -1276,4 +1276,4 @@ | |||
| "vscode-languageclient": "^9.0.1", | |||
| "xml2js": "^0.6.2" | |||
| } | |||
| } | |||
There was a problem hiding this comment.
nit: each PR keeps changing this. Think there is an NPMRC setting to set consistent line ending
There was a problem hiding this comment.
Will create a separate pull request to enforce this behaviour.
|
@swift-server-bot test this please |
|
I've reverted back to using CodeLLDB by default for this patch until we can verify that lldb-dap has feature parity with CodeLLDB. |
The CodeLLDB extension expects the debug adapter to be called `lldb`. If we are using it and we don't call `registerDebugAdapterTrackerFactory` with `lldb` as the adapter name then we fail to get logs in the debugSessionCallback when running tests. This prevents XCTests results from being parsed, and hides the swift-testing outout. Revert a portion of swiftlang#1024 that always set the debug adapter name to `swift-lldb` and use `lldb` when we're using CodeLLDB as the debug adapter. Issue swiftlang#1100
The CodeLLDB extension expects the debug adapter to be called `lldb`. If we are using it and we don't call `registerDebugAdapterTrackerFactory` with `lldb` as the adapter name then we fail to get logs in the debugSessionCallback when running tests. This prevents XCTests results from being parsed, and hides the swift-testing outout. Revert a portion of #1024 that always set the debug adapter name to `swift-lldb` and use `lldb` when we're using CodeLLDB as the debug adapter. Issue #1100
Users should not have to know which debug adapter is being used (CodeLLDB vs
lldb-dap). The Swift extension will now always create aswift-lldblaunch config for each executable that it finds in the workspace. When launched these configurations will either delegate to CodeLLDB or launchlldb-dapdepending on which Swift toolchain version is currently being used. I've added a setting to force the usage of CodeLLDB when desired.Issue: #1013
Additionally, the
programpath in the launch configuration will always use POSIX style paths regardless of the platform. This allows the launch.json to be shared between users of different platforms.Issue: #958