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

Overriding default debugOptions in launch.json #1326

Closed
DonJayamanne opened this issue Apr 6, 2018 · 2 comments · Fixed by #1395
Closed

Overriding default debugOptions in launch.json #1326

DonJayamanne opened this issue Apr 6, 2018 · 2 comments · Fixed by #1395
Assignees
Labels
area-debugging feature-request Request for new features or functionality
Milestone

Comments

@DonJayamanne
Copy link

DonJayamanne commented Apr 6, 2018

Currently we have debug options defined in debugOptions setting of launch.json file.
Here are some of those settings:

    "debugOptions":  [
            "RedirectOutput",       // Whether to redirect stdout and stderr (see pydevd_comm.CMD_REDIRECT_OUTPUT)
            "Django",               // Enables Django Template debugging
            "Jinja",                // Enables Jinja (Flask) Template debugging
            "FixFilePathCase",      // Used to ensure file paths on windows are properly cased (as they exist on the fs)
            "DebugStdLib"           // Whether to enable debugging of standard library functions
            "UseSourceReferences"   // New feature (#1322)
    ],

Now, the defaults:

  • When you create a launch.json, debugOptoins is not in the file (except for some configurations, e.g. django, flask)
  • We always inject RedirectOutput (this was a decision made in a previous version of the extension)
  • Now we have a new feature to enable source references (Add support for remote source references #1322), and I think we should just inject UseSourceReferences into debugOptions (always)
  • If the user is debugging a flask app ("module": "flask" in launch.json), then we inject Jinja into debugOptions

This is all well and good, the problem is there's no way for the user to opt out of these (injected) defaults. E.g. the user for some reason does not want the output to be redirected to the Debug Console, or they do not want source references enabled by default. I cannot think of any reason why one wouldn't want these, but I'm concerned about the way we're forcing these defaults. The user has no way of opting out of these.

The only solution I can think of, is to use individual settings in launch.json as follows:

{
            "redirectOutput": true/false,
            "django": true/false,
            "jinja": true/false,
            "fixFilePathCase": true/false
}

This way, if the values are not in launch.json we use the defaults, else user can chose to opt out by providing values for them with the value false in launch.json

However, this is inconsistent compared to the previous version of debugger.

@qubitron @brettcannon

@DonJayamanne DonJayamanne added this to the April 2018 milestone Apr 6, 2018
@DonJayamanne DonJayamanne self-assigned this Apr 6, 2018
@patrys
Copy link

patrys commented Apr 6, 2018

Would it be possible to use the importlib hooks to detect Django/Jinja as they are imported?

@DonJayamanne
Copy link
Author

importlib hooks to detect Django/Jinja as they are imported?

This was discussed, however this just introduces unnecessary complexity and with a higher probability of things to going wrong.

@brettcannon brettcannon added bug Issue identified by VS Code Team member as probable bug area-debugging needs decision feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug needs decision labels Apr 9, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-debugging feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants