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 Vakhurin Sergei (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

Microsoft Contribution License Agreements (msftclas) commented Sep 7, 2017

Copy link
Copy Markdown

Vakhurin Sergei (@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 suggested changes Sep 8, 2017
View reviewed changes

@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

Vakhurin Sergei (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

roblourens commented Sep 8, 2017

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

roblourens commented Sep 11, 2017

Copy link
Copy Markdown
Member

Thanks!

@roblourens roblourens added this to the September 2017 milestone Sep 29, 2017
@dboreham David Boreham (dboreham) mentioned this pull request Nov 18, 2020
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@roblourens roblourens roblourens approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

September 2017

Development

Successfully merging this pull request may close these issues.

3 participants

@igelbox @msftclas @roblourens