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

Implement debug target launching in integrated terminal #10574

Closed
weinand opened this issue Aug 16, 2016 · 14 comments
Closed

Implement debug target launching in integrated terminal #10574

weinand opened this issue Aug 16, 2016 · 14 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality

Comments

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

This is the frontend implementation for microsoft/vscode-debugadapter-node#47.
The implementation for node-debug is this: microsoft/vscode-node-debug#85
The implementation would be based on #9957.

@weinand weinand added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues labels Aug 16, 2016
@weinand weinand added this to the September 2016 milestone Aug 16, 2016
@weinand weinand changed the title Implement debug target launching in internal terminal Implement debug target launching in integrated terminal Aug 16, 2016
@weinand weinand removed this from the September 2016 milestone Aug 23, 2016
@weinand
Copy link
Contributor Author

weinand commented Aug 23, 2016

@isidorn I've added a first cut of this to the debug protocol and node-debug.
If you pass a supportsRunInTerminalRequest with value true to the InitializeRequest, node_debug will call the runInTerminalRequest on VS Code. See https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/node/v8Protocol.ts#L69

@Tyriar
Copy link
Member

Tyriar commented Aug 23, 2016

Keep in mind that the command sent would be sent to the currently configured shell (bash, zsh, powershell, cmd, etc.).

@DanTup
Copy link
Contributor

DanTup commented Aug 24, 2016

@weinand I'm not familiar with/haven't installed the Insiders builds yet - that change is in master, does that mean we can try it out in insiders from tomorrow? :)

(Also, how releases are cut; presumably not from master if there's in-progress stuff like this that might not make the release? I searched but couldn't find any info on the relationship between branches/releases/etc.)

@weinand
Copy link
Contributor Author

weinand commented Aug 24, 2016

@DanTup you can try this as soon as this issue is closed. Currently the VS Code side of things is missing (and we are not sure that this will be implemented in the August release).

Releases are cut from Master. In-progress stuff will either be disabled or just not mentioned in the release notes.

@isidorn
Copy link
Contributor

isidorn commented Aug 24, 2016

Released a first version of this. Please note the following:

  • I am not using the Terminal extension API since vscode debugger UI is not running as an extension, so I am accessing the TerminalService directly
  • Currently ignoring: env, cwd and processID

@weinand
Copy link
Contributor Author

weinand commented Aug 24, 2016

@DanTup before you jump on this: please note the limitations mentioned above. I'll work on this via #10880

@isidorn
Copy link
Contributor

isidorn commented Aug 24, 2016

Also externalConsole now might not be the best name since the intergrated terminal is not external.

@weinand
Copy link
Contributor Author

weinand commented Aug 28, 2016

@isidorn For node-debug I've introduced a new console attribute with these values:
"internalConsole", "integratedTerminal", "externalTerminal"

The externalConsole attribute has been deprecated in hover help. If it is still used, a value of true is identical to "console": "externalTerminal".

@DanTup
Copy link
Contributor

DanTup commented Aug 29, 2016

@weinand This sounds great; we'll give it a go! With microsoft/vscode-node-debug#85 closed does this mean there's a working example of this for Node runnable in insiders from Monday?

@weinand
Copy link
Contributor Author

weinand commented Aug 29, 2016

@DanTup yep, give it a try...

@DanTup
Copy link
Contributor

DanTup commented Aug 31, 2016

@weinand Had a quick go, and this seems to work great!

One question though - the supportsRunInTerminal flag - when would it ever be false? If it's only for <= Code 1.4 is there a way we can flag the extension to only run in >=1.5? (I'd rather ditch that code path if we don't need it, but I can't figure it out).

@weinand
Copy link
Contributor Author

weinand commented Aug 31, 2016

@DanTup great to hear that runInTerminal works.
The flag supportsRunInTerminal does not exist in VS Code <= 1.4, so prepare for this (and not that it is false).
You can set the vscode engine version in the package.json to 1.5:

    "engines": {
        "vscode": "^1.5.0",
        "node": "^4.1.1"
    },

@DanTup
Copy link
Contributor

DanTup commented Aug 31, 2016

@weinand I had updated engines/vscode to 1.5 but I wasn't sure if that would stop people being able to install in 1.4 - is that correct? If so, it's safe for me the to assume integrated terminal support always?

@weinand
Copy link
Contributor Author

weinand commented Aug 31, 2016

@DanTup yes, that should stop people installing your extension in 1.4.
But I've never tried that myself, so I suggest that your try it to better understand what users of your extension will experience.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants