[Lldb-commits] [lldb] [lldb-dap] Added "port" property to vscode "attach" command. (PR #91570)

[Pavel Labath via lldb-commits]

labath wrote:

> > FWIW, I think this feature would be more useful and easier to implement if there was just a single setting called "connection url" or something, which accepted all of the usual lldb connection urls (unix-abstract-connect://, listen://, ...).
> > Since a simple tcp port connection is definitely going to be the most commonly used connection, you can, if you want, treat a simple number (or `:number`) as an alias to `connect://localhost:number`. lldb-server does something similar.
> > PS: I'm clicking request changes because I don't believe the test for the feature can go in in this form. I don't consider myself an owner for the rest, so you can consider my comments as suggestions.
> 
> I apologize for missing your earlier comment. To clarify. Using the URL "connect://localhost:12345" or "connect://:12345" with the "connect" scheme helps invoke the appropriate function to establish a connection. Pls ref: https://github.com/llvm/llvm-project/blob/main/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp#L148 $gdb-remote number - would work from the command-line, because lldb is taking care of adding "connect://".
> 
> Please let me know if my understating is incorrect.

Your understanding of the code is correct. What I am suggesting is basically to emulate this command line behavior in vscode. 

Basically, instead of two host+port settings, you could have one setting with a sufficiently vague name ("connection url", "remote address", ...) to cover the expanded scope. Then, you implementation could take the user-provided string and fill in the blanks:
- if the user types "1234", you'd convert it to `connect://localhost:1234`
- if the user types "my-host:1234", you'd convert it to `connect://localhost:1234`
- if the user types "unix-abstract-connect://..." (or any other scheme supported by lldb), you'd pass the value unmodified

The advantage of this is that you'd automatically support every connection protocol that lldb understands with almost no effort. If we stick to the current implementation, then if someone later needs to connect through unix sockets (or whatever), he would have to go back and add *another* setting to support connecting through that.

Now, I do see the appeal of having an explicit host/port fields (they're much easier to understand for the average user), which is why I'm not insisting on this approach. Nonetheless, I think this is an option worth considering, and I think we could avoid most of the confusion by carefully crafting the description of this field to guide the user to the most common scenarios.

https://github.com/llvm/llvm-project/pull/91570