-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Use new environment variable parser #362
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
Changes from 135 commits
ecc1ca9
7c5778c
9d1bf82
ffba179
905c713
acc2109
d470523
d32a546
3f14d0a
f37803f
d69940e
d392e8b
11c9514
38befc0
bea91fc
0bb9054
a105a20
8b2fd7c
5a7d21e
acd90c5
09b3359
9616eef
f9fe2a8
4963632
edbe63e
47acd08
284e221
b27496b
92f775f
23fe3da
0277ca1
bbc8290
29b5489
5e7a61b
81af0e5
e0ea914
f0c1978
32a6e53
4b30f2c
e396752
4c0bdb1
3b71463
b5d9b1a
56ce88b
eff4792
4553c28
6382e96
8ab2eb5
39f272f
b05a940
5049ed9
3c6520a
966e516
63d2d65
f6d469e
eced312
ce930dc
53bbc1e
f0c4714
db0e778
024509d
a32f846
83a1ce5
560752b
1addf10
73c7c81
07a8a18
b4f5c10
f14df93
e856fe8
029e055
f0ba6d7
6dcf879
71c2531
e8c71c0
b5c51c4
7e6ee4c
51cf9d2
13df41b
4968ebd
7aadc43
aa4062e
9ef2465
0e15e8d
5a60422
5cfb06f
b2b7dc6
35b8255
7854924
d69fd1a
317cf88
07e0710
9e0d27a
5a22791
fec7d51
0c4003f
9d6194b
34158ba
c823315
e349e56
a1fea85
cb0a354
f0f5c59
c464a2a
011dca1
bfb8036
761f057
b2b9da9
51b6c81
7fd36fc
ada46ba
c5e2d5d
b756ee8
30a4091
de14c78
22e90fb
b16d2f9
d1079a2
c0a2787
479a3c2
514f654
2b3f2ef
51de97b
1c7d1c9
c0e8d8f
7bc71b0
c8db345
d884256
041ae0c
248575e
0df7f16
67387bf
b5901d8
5785bec
679329b
a9edca8
554688b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,11 @@ import * as path from 'path'; | |
import { DebugSession, OutputEvent } from 'vscode-debugadapter'; | ||
import { DebugProtocol } from 'vscode-debugprotocol'; | ||
import { open } from '../../common/open'; | ||
import { EnvironmentVariablesService } from '../../common/variables/environment'; | ||
import { EnvironmentVariables } from '../../common/variables/types'; | ||
import { IDebugServer, IPythonProcess } from '../Common/Contracts'; | ||
import { LaunchRequestArguments } from '../Common/Contracts'; | ||
import { getCustomEnvVars } from '../Common/Utils'; | ||
import { IS_WINDOWS } from '../Common/Utils'; | ||
import { BaseDebugServer } from '../DebugServers/BaseDebugServer'; | ||
import { LocalDebugServer } from '../DebugServers/LocalDebugServer'; | ||
import { DebugClient, DebugType } from './DebugClient'; | ||
|
@@ -18,9 +20,9 @@ const VALID_DEBUG_OPTIONS = [ | |
'DjangoDebugging']; | ||
|
||
export class LocalDebugClient extends DebugClient { | ||
protected pyProc: child_process.ChildProcess; | ||
protected pyProc: child_process.ChildProcess | undefined; | ||
protected pythonProcess: IPythonProcess; | ||
protected debugServer: BaseDebugServer; | ||
protected debugServer: BaseDebugServer | undefined; | ||
// tslint:disable-next-line:no-any | ||
constructor(args: any, debugSession: DebugSession, private canLaunchTerminal: boolean) { | ||
super(args, debugSession); | ||
|
@@ -38,24 +40,24 @@ export class LocalDebugClient extends DebugClient { | |
|
||
public Stop() { | ||
if (this.debugServer) { | ||
this.debugServer.Stop(); | ||
this.debugServer = null; | ||
this.debugServer!.Stop(); | ||
this.debugServer = undefined; | ||
} | ||
|
||
if (this.pyProc) { | ||
try { | ||
this.pyProc.send('EXIT'); | ||
this.pyProc!.send('EXIT'); | ||
// tslint:disable-next-line:no-empty | ||
} catch { } | ||
try { | ||
this.pyProc.stdin.write('EXIT'); | ||
this.pyProc!.stdin.write('EXIT'); | ||
// tslint:disable-next-line:no-empty | ||
} catch { } | ||
try { | ||
this.pyProc.disconnect(); | ||
this.pyProc!.disconnect(); | ||
// tslint:disable-next-line:no-empty | ||
} catch { } | ||
this.pyProc = null; | ||
this.pyProc = undefined; | ||
} | ||
} | ||
protected getLauncherFilePath(): string { | ||
|
@@ -71,7 +73,8 @@ export class LocalDebugClient extends DebugClient { | |
} | ||
} | ||
// tslint:disable-next-line:max-func-body-length member-ordering no-any | ||
public LaunchApplicationToDebug(dbgServer: IDebugServer, processErrored: (error: any) => void): Promise<any> { | ||
public async LaunchApplicationToDebug(dbgServer: IDebugServer, processErrored: (error: any) => void): Promise<any> { | ||
const environmentVariables = await this.getEnvironmentVariables(); | ||
// tslint:disable-next-line:max-func-body-length cyclomatic-complexity no-any | ||
return new Promise<any>((resolve, reject) => { | ||
const fileDir = this.args && this.args.program ? path.dirname(this.args.program) : ''; | ||
|
@@ -83,8 +86,6 @@ export class LocalDebugClient extends DebugClient { | |
if (typeof this.args.pythonPath === 'string' && this.args.pythonPath.trim().length > 0) { | ||
pythonPath = this.args.pythonPath; | ||
} | ||
let environmentVariables = getCustomEnvVars(this.args.env, this.args.envFile, false); | ||
environmentVariables = environmentVariables ? environmentVariables : {}; | ||
if (!environmentVariables.hasOwnProperty('PYTHONIOENCODING')) { | ||
environmentVariables.PYTHONIOENCODING = 'UTF-8'; | ||
} | ||
|
@@ -103,12 +104,16 @@ export class LocalDebugClient extends DebugClient { | |
break; | ||
} | ||
default: { | ||
// As we're spawning the process, we need to ensure all env variables are passed. | ||
// Including those from the current process (i.e. everything, not just custom vars). | ||
const envParser = new EnvironmentVariablesService(IS_WINDOWS); | ||
envParser.mergeVariables(process.env as EnvironmentVariables, environmentVariables); | ||
this.pyProc = child_process.spawn(pythonPath, args, { cwd: processCwd, env: environmentVariables }); | ||
this.handleProcessOutput(this.pyProc, reject); | ||
this.handleProcessOutput(this.pyProc!, reject); | ||
|
||
// Here we wait for the application to connect to the socket server. | ||
// Only once connected do we know that the application has successfully launched. | ||
this.debugServer.DebugClientConnected | ||
this.debugServer!.DebugClientConnected | ||
.then(resolve) | ||
.catch(ex => console.error('Python Extension: debugServer.DebugClientConnected', ex)); | ||
} | ||
|
@@ -120,10 +125,10 @@ export class LocalDebugClient extends DebugClient { | |
proc.on('error', error => { | ||
// If debug server has started, then don't display errors. | ||
// The debug adapter will get this info from the debugger (e.g. ptvsd lib). | ||
if (!this.debugServer && this.debugServer.IsRunning) { | ||
if (!this.debugServer && this.debugServer!.IsRunning) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I understand the syntax. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
return; | ||
} | ||
if (!this.debugServer.IsRunning && typeof (error) === 'object' && error !== null) { | ||
if (!this.debugServer && !this.debugServer!.IsRunning && typeof (error) === 'object' && error !== null) { | ||
return failedToLaunch(error); | ||
} | ||
// This could happen when the debugger didn't launch at all, e.g. python doesn't exist. | ||
|
@@ -136,7 +141,7 @@ export class LocalDebugClient extends DebugClient { | |
|
||
// Either way, we need some code in here so we read the stdout of the python process, | ||
// Else it just keep building up (related to issue #203 and #52). | ||
if (this.debugServer && !this.debugServer.IsRunning) { | ||
if (this.debugServer && !this.debugServer!.IsRunning) { | ||
return failedToLaunch(error); | ||
} | ||
}); | ||
|
@@ -193,12 +198,32 @@ export class LocalDebugClient extends DebugClient { | |
this.pyProc = proc; | ||
resolve(); | ||
}, error => { | ||
if (!this.debugServer && this.debugServer.IsRunning) { | ||
if (!this.debugServer && this.debugServer!.IsRunning) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also may be worth encapsulating the condition into a small helper function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
return; | ||
} | ||
reject(error); | ||
}); | ||
} | ||
}); | ||
} | ||
private async getEnvironmentVariables(): Promise<EnvironmentVariables> { | ||
const args = this.args as LaunchRequestArguments; | ||
const envParser = new EnvironmentVariablesService(IS_WINDOWS); | ||
const envFileVars = await envParser.parseFile(args.envFile); | ||
|
||
const hasEnvVars = args.env && Object.keys(args.env).length > 0; | ||
if (!envFileVars && !hasEnvVars) { | ||
return {}; | ||
} | ||
if (envFileVars && !hasEnvVars) { | ||
return envFileVars!; | ||
} | ||
if (!envFileVars && hasEnvVars) { | ||
return args.env as EnvironmentVariables; | ||
} | ||
// Merge the two sets of environment variables. | ||
const env = { ...args.env } as EnvironmentVariables; | ||
envParser.mergeVariables(envFileVars!, env); | ||
return env; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth making a small helper like
getPathVeriableName()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, now i can create the PathUtils class I've always wanted (to translate HOME and other environment varaibels and back)... for prettier displays of paths..