-
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
dap-python: support debugging of 3rd modules #386
base: master
Are you sure you want to change the base?
Conversation
- Add configuration handling to support step into 3rd modules. - justMyCode is the single configuration needed to be specified regardless the selected back end debugger. - DebugStdLib is also supported for backward compatibility, and a warning message will be printed suggesting for a migration if debugpy is selected as the debugger.
@@ -105,6 +106,7 @@ settings. | |||
:cwd nil | |||
:env '(("DEBUG" . "1")) | |||
:target-module (expand-file-name "~/src/myapp/.env/bin/myapp") | |||
:justMyCode :json-false |
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.
:justMyCode :json-false | |
:justMyCode nil |
:justMyCode
should be optional and off by default. Also, use, and make sure the user can use, nil
in such cases since that is more idiomatic in ELisp, which lacks a dedicated false type. json-encoder-specifics shouldn't leak trough.
(sequencep debug-options) | ||
(seq-contains debug-options "DebugStdLib") | ||
(null justmycode)) | ||
(warn "DebugStdLib is deprecated, use justMyCode, please consider to update your configuration.") |
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.
Why the backwards compatibility considerations here? :debugOptions
is introduced with this PR, so there cannot be a config in the wild using it.
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'd just drop that warning altogether. You could state in the README somewhere that :debugOptions is ptsvd
-specific, and that :justMyCode
is the preferred way to go about this, since it is portable.
@@ -216,7 +218,12 @@ overriden in individual `dap-python' launch configurations." | |||
(plist-put conf :debugServer debug-port) | |||
(plist-put conf :port debug-port) | |||
(plist-put conf :hostName host) | |||
(plist-put conf :host host))) | |||
(plist-put conf :host host) | |||
(when (and (not (null justmycode)) |
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.
(when (and (not (null justmycode)) | |
(when (and justmycode |
Chaining not
and null
if only the result's truthynesss matters is redundant and non-idiomatic.
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.
FYI, not
and null
are aliases.
(when (and (not (null justmycode)) | ||
(equal justmycode :json-false)) | ||
(setq debug-options (cl-pushnew "DebugStdLib" debug-options :test #'equal)) | ||
(plist-put conf :debugOptions debug-options) |
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.
This will create a duplicate if :debugOptions is specified, with this version being overriden (it will show up twice in the JSON communcation). '(:debugOptions ...) -> '(:debugOptions (DebugStdLib . ...) :debugOptions ...)
My general suggestion here is: for |
regardless the selected back end debugger.
a warning message will be printed suggesting for a migration
if debugpy is selected as the debugger.