-
Notifications
You must be signed in to change notification settings - Fork 183
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
Enhance debugger attachment requests #2431
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.
I just have an ergonomics comment, but it looks good to me
"debugPort": { | ||
"type": "number", | ||
"description": "The port to use to connect to the debugger" | ||
}, | ||
"debugHost": { | ||
"type": "string", | ||
"description": "The host to use to connect to the debugger" | ||
} |
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.
Can we combine these two into a single URI? I think it makes it even simpler for the user and we can easily split the host and port by using VS Code's URI API:
const uri = vscode.Uri.parse(session.configuration.debugUri)
uri.host
uri.port
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.
I prefer having them separated because:
- When connecting with a remote console, the usage is
rdbg -A host port
, notrdbg -A host:port
. When specifying with env, it hasRUBY_DEBUG_HOST
andRUBY_DEBUG_PORT
but noRUBY_DEBUG_URI
either. So I think having them separated is more consistent with the terminal experience. port
is going to be specified a lot more often thanhost
, which means in most casesdebugUri
would simply beport
, likedebugUri: 4000
. And IMOdebugPort: 4000
is a lot less confusing than it.
Motivation
Port and host support closes #1787
Socket path support is required for https://github.com/Shopify/team-ruby-dx/issues/926
Implementation
Automated Tests
Manual Tests
debugSocketPath
:RUBY_DEBUG_SOCK_PATH=/tmp/debug-socket-test bundle exec rdbg -O -n -c -- bin/rails s
debugSocketPath
succeeded:debugPort
anddebugHost
:bundle exec rdbg -p 1234 -O -n -c -- bin/rails s
debugPort
succeeded:debugHost
failed: