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

Add support for runInTerminal request #67

Closed
wants to merge 1 commit into from
Closed

Conversation

felixfbecker
Copy link
Contributor

@felixfbecker felixfbecker commented Oct 12, 2016

Closes #60

  • externalConsole is now deprecated
  • you can now run your program in the integrated terminal

@felixfbecker
Copy link
Contributor Author

@weinand Does the testsupport already support the runInTerminal request?

@weinand
Copy link

weinand commented Oct 13, 2016

@felixfbecker not yet. Since this is the first 'reverse' request, it needs some work to provide support for this...

@felixfbecker
Copy link
Contributor Author

@weinand that's unfortunate, I thought that it is implemented in the node debugger would mean you also have tests for that. Cannot merge this because it seems to break the tests completely even though everything works fine.

@weinand
Copy link

weinand commented Oct 13, 2016

@felixfbecker I do not understand what you mean and why it breaks something.
The runInTerminal is implemented in VS Code and node-debug is calling it. The test support is independent from VS Code so it has to implement 'runInTerminal' itself.

@felixfbecker
Copy link
Contributor Author

runInTerminal is implemented in node-debug. So I assumed you had tests for that in the test suite of the node-debug, so I assumed the testsupport would support it.

The failing tests were a bug on my side though, sorry. I just don't feel very confident merging this as I can't write any tests without testsupport...

@felixfbecker
Copy link
Contributor Author

@weinand Is there a way to highlight the externalConsole attribute as deprecated like for the Node debugger? Something like "deprecated": true in the JSON schema?

@codecov-io
Copy link

codecov-io commented Oct 13, 2016

Codecov Report

Merging #67 into master will increase coverage by 12.74%.
The diff coverage is 84.37%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #67       +/-   ##
===========================================
+ Coverage   62.28%   75.03%   +12.74%     
===========================================
  Files           5        4        -1     
  Lines         830      733       -97     
  Branches      138      125       -13     
===========================================
+ Hits          517      550       +33     
+ Misses        256      182       -74     
+ Partials       57        1       -56
Impacted Files Coverage Δ
src/phpDebug.ts 66.66% <84.37%> (+8.09%) ⬆️
src/xdebugConnection.ts 81.11% <0%> (+3.43%) ⬆️
src/paths.ts 100% <0%> (+5.71%) ⬆️
src/dbgp.ts 94.82% <0%> (+12.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02acaf3...1a19fc3. Read the comment docs.

@zobo
Copy link
Contributor

zobo commented Jun 30, 2024

Closing in favor of #966

@zobo zobo closed this Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runInTerminal request
4 participants