The Wayback Machine - https://web.archive.org/web/20201202202623/https://github.com/microsoft/vscode/issues/109216
Skip to content

/ vscode

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

Jump to bottom

External terminal not logging Electron logs #109216

Open
laggingreflex opened this issue Oct 23, 2020 · 6 comments · May be fixed by #109933
Open

External terminal not logging Electron logs #109216

laggingreflex opened this issue Oct 23, 2020 · 6 comments · May be fixed by #109933
Assignees
@Tyriar
Labels
bug good first issue help wanted
Milestone
Backlog

Comments

@laggingreflex
Copy link

@laggingreflex laggingreflex commented Oct 23, 2020 •
edited

Version: 1.50.1 (system setup)
Commit: d2e414d
Date: 2020-10-13T15:06:15.712Z
Electron: 9.2.1
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Windows_NT x64 10.0.18363

Steps to Reproduce:

  1. Create a sample Electron project.

    In the main process log to console a sample text line

    const { app, BrowserWindow } = require('electron')
...
console.log('SAMPLE TEXT LINE')

  2. Open VSCode

  3. Launch external terminal CtrlShiftC

  4. Run the electron app

    electron .

Expected:

It should log

electron .
SAMPLE TEXT LINE

Actual:

Nothing is logged

electron .

Does this issue occur when all extensions are disabled?: Yes
@Tyriar
Copy link
Member

@Tyriar Tyriar commented Oct 30, 2020

It looks like the environment is probably getting inherited from VS Code which could cause this as it's also an Electron app. We could try sanitizing the env:

vscode/src/vs/base/common/processes.ts

Line 104 in cf4b5a7

export function sanitizeProcessEnvironment(env: IProcessEnvironment, ...preserve: string[]): void {

Here:

vscode/src/vs/workbench/contrib/externalTerminal/node/externalTerminalService.ts

Lines 101 to 102 in cf4b5a7

const env = cwd ? { cwd: cwd } : undefined;
const child = spawner.spawn(command, cmdArgs, env);
@Tyriar Tyriar added this to the Backlog milestone Oct 30, 2020
@keremgocen
Copy link

@keremgocen keremgocen commented Oct 30, 2020

up for grabs I assume?
@Tyriar
Copy link
Member

@Tyriar Tyriar commented Oct 30, 2020

@keremgocen yep
@patlehmann1
Copy link

@patlehmann1 patlehmann1 commented Oct 31, 2020

I'm willing to open a PR for this, would just need some background as I am completely green to this project. Would anyone be available to walk me through some things?
@Tyriar
Copy link
Member

@Tyriar Tyriar commented Nov 2, 2020

@patlehmann1 the basic changes needed are outlined here (note there are 2 places where spawn is called under Windows):

#109216 (comment)

Contributing docs are here: https://github.com/Microsoft/vscode/wiki/How-to-Contribute
jaykayitare added a commit to jaykayitare/vscode that referenced this issue Nov 3, 2020
@jaykayitare
microsoft#109216 Sanitize Process Environment before spawning termina…
f3db8ba 
…l instance
@jaykayitare jaykayitare linked a pull request that will close this issue Nov 3, 2020
Open
@rolimans
Copy link

@rolimans rolimans commented Nov 17, 2020

Is this issue being take care of? I'm looking to start contributing and thought I may help
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Tyriar Tyriar

Labels
bug good first issue help wanted
Projects
None yet
Milestone
Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

Sanitize Process Environment before spawning terminal instance
5 participants
@keremgocen @Tyriar @laggingreflex @rolimans @patlehmann1
You can’t perform that action at this time.