Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Add a launch.json option to capture output from stdoutput and stderr streams#138

Merged
roblourens merged 1 commit into
microsoft:masterfrom
igelbox:add-outputcapture-option
Sep 11, 2017
Merged

Add a launch.json option to capture output from stdoutput and stderr streams#138
roblourens merged 1 commit into
microsoft:masterfrom
igelbox:add-outputcapture-option

Conversation

@igelbox

@igelbox igelbox commented Sep 7, 2017

Copy link
Copy Markdown
Contributor

This will help with microsoft/vscode#19750.
We'll just set "outputCapture": "std", in the launch.json file.

...
  "configurations": [{
    "name": "Debug Jasmine",
    "type": "node",
    ...
    "debugCapture": "std",
    ...
  }
...

This approach is more lightweight than the original proposed "console": "internalConsole" approach, which starts a separate shell process with (e.g. on my PC zsh with lots of plugins is started).

@msftclas

msftclas commented Sep 7, 2017

Copy link
Copy Markdown

@igelbox,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@roblourens roblourens left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great, thanks!

When capturing stdout/err, I think it shouldn't show messages that come over the debug port, to avoid showing duplicate messages. Can you override onConsoleAPICalled to disable that in this case?

@igelbox

igelbox commented Sep 8, 2017

Copy link
Copy Markdown
Contributor Author

@roblourens thanks for your feedback.
I didn't quite get your point. The onConsoleAPICalled is already overridden in this class, so I've just added a conditional return statement there to prevent duplicate messages.

@roblourens

Copy link
Copy Markdown
Member

You are quite right. Sorry, I misread... I will merge this tomorrow.

@roblourens roblourens merged commit 9b1e72b into microsoft:master Sep 11, 2017
@roblourens

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants