No way to configure an adapter that is not hardcoded into the plugin?
justarandomgeek opened this issue · 8 comments
I was trying to help a user get set up with Factorio Modding Tool Kit's Debug Adapter (via a global install of the npm package), but after we got it installed we got stumped because it seems there is no way to configure this plugin to use an adapter that it does not itself install?
I'm normally a VSCode user, so maybe I'm missing something by my unfamiliarity with sublime, but surely loading other adapters is the whole point of a DAP client?
AdapterConfiguration is how things are setup and registered and ideally all exist in this package so other users can benefit from them. There is no public api and I see little benefit and lots of downsides to splitting things up into a bunch of helper packages like LSP if thats what you are suggesting it just makes things more complicated.
Most adapters here just download the vscode extension since they usually include the adapter as well as snippets and json schema support but thats not strictly required
In FMTK's case, the vscode extension does more workspace-aware config tweaking to the command to run, to support an in-editor factorio version picker, so the definition form the extension is not quite sufficient by itself. For other editors i've helped users with, they could just point it an an arbitrary program to run as the DAP adapter, so we could fill in the appropriate command line options there. If you aren't loading the whole vscode extension for the in-editor factorio version picker (or an equivalent replacement), then you'll have to provide somewhere for users to customize the adapter's command line.
If I wanted to install a DAP for Playdate, I would have to clone this repo and write an adapter for it, right? That's kind of the sticking point with this issue, if I understand it. SublimeDebugger cannot be used as a dependency for other debug adapter packages.
Most adapters here just download the vscode extension since they usually include the adapter as well as snippets and json schema support but thats not strictly required
Sure, but the rest of the VSCode extension is a bunch of TypeScript or whatever that is just ignored. The adapter needs to at least attach to the server through stdio or TCP. It seems like a starting point for a compromise would be providing a "generic TCP adapter" and a "generic stdio adapter" that can be used with minimal configuration. At least until someone wants to dive in and develop a more feature-rich adapter in a fork.
The protocol doesn't specify how to start to start or communicate with the adapter which makes generic tcp/stdio configurations not even remotely powerful enough.
Its like 10 lines of code to configure an adapter outside of this package for testing.
Plugin
|-- .python-version -> 3.8
|-- plugin.py -> ...
from Debugger.modules import dap, core
from Debugger.modules.adapters_registry import AdaptersRegistry
class Adapter(dap.AdapterConfiguration):
type = 'adapter-type'
async def start(self, log: core.Logger, configuration: dap.ConfigurationExpanded):
return await dap.SocketTransport.connect_with_retry(log, 'localhost', 1234)
AdaptersRegistry.register(Adapter())Sure, but the rest of the VSCode extension is a bunch of TypeScript or whatever that is just ignored
You're talking about an irrelevant amount of space here and the alternative is me building and packaging most adapters which isn't going to happen
You're talking about an irrelevant amount of space here and the alternative is me building and packaging most adapters which isn't going to happen
It is irrelevant to space, but it's functionality that is necessary for VSCode. Functionality which you have to reproduce in this plugin regardless. So, you've already done the work of building and packaging most adapters. Not sure I see the point you are making. But the point I'm making is that I have to add those 10 lines if I want to add a new adapter and there is no alternative.
Those packages include a lot more than just debugging support and they have snippets and schemas for LSP and have compiled the servers (sometimes for multiple platforms) so no the AdapterConfiguration implementations are not even remotely close to the work that would be required from me to setup tooling to build and package all the adapters.
When its 10 lines of code I do not see the need to add an alternative that won't work for most dap servers anyway
Are you talking about packaging servers? I don't think it's relevant, but I agree that duplicating that work would be silly.
In the case of Playdate, I do not want the plugin to install the Playdate SDK (which ships the simulator that implements the server). It feels like we are not talking about the same thing. All I want is a way to connect to some server that is already running somewhere else, and I do not need the plugin to manage that server. The server could be running on a separate machine, for instance.
But the crux is that I have to add a new adapter to this plugin every time I want to support a new debugger. And then I need to create a PR to upstream them all, and hope that the PRs are accepted and not considered "trivial" because it's just 10 lines of code. There is a real issue, here.
I just told you how to add a adapter configuration outside of this package if thats what you wanted to do.
You are acting like connecting to an already running server on specific port is the usual case but its not. The docs you link to don't even tell you how to use their adapter and don't even mention that it is running on a specific port. No one is expecting users to manually use the adapters and thats not how VSCode or this package is designed to work.
There are multiple very simple adapter configurations supported in this project such as the ruby adapter which expects the user to have installed rdbg. The only reason it downloads the vscode package is so there are snippets and an LSP-json schema for debugger_configurations which are automatically extracted from the package.json file.
If you add support for an adapter your should probably make a pull request even if its "trivial" so other users don't need to figure out how to configure the adapter thats the entire point of this package.