Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

f279801
Copy link
Contributor

@f279801 f279801 commented Oct 13, 2020

  • 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.

- 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: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.")
Copy link
Member

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.

Copy link
Member

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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(when (and (not (null justmycode))
(when (and justmycode

Chaining not and null if only the result's truthynesss matters is redundant and non-idiomatic.

Copy link
Member

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)
Copy link
Member

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 ...)

@nbfalcon
Copy link
Member

My general suggestion here is: for debugpy, nothing else needs to be done (i.e.: no depreciation warnings, ...). :debugOptions should just be ignored, and not even removed. :justMyCode is alreay handled, implicitly for debugpy. For ptsvd, you can add additional support for :justMyCode. For this, you'll need to plist-get it, and to cl-remf it if justMyCode is explicitly specified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants