Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Add support for reading http.proxy setting #639

Merged
merged 2 commits into from
Dec 30, 2016
Merged

Add support for reading http.proxy setting #639

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b99e87
Add support for reading http.proxy setting
ibigbug Nov 22, 2016
9f210f7
fix proxy config
ibigbug Dec 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/goInstallTools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ function installTools(goVersion: SemVersion, missing?: string[]) {

outputChannel.appendLine(''); // Blank line for spacing.

let http_proxy = vscode.workspace.getConfiguration('http').get('proxy');
let https_proxy = http_proxy; // default proxy for http & https
Copy link
Contributor

Choose a reason for hiding this comment

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

@ibigbug Sorry for the very late review.

Have you tested the case where http_proxy is not set in settings , but is set in the environment variable?
In that case will the value set in the environment variable make its way to the child process running the go get command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramya-rao-a Yeah. I'm sure http_proxy in environment variables and open VS code from this environment using $ code can make go get work through proxy, though it's limited use case. User may often not open VS code through terminal and for OS X users (like me) and other Linux users, environment variables for terminal and GUI are always isolated, this is the main consideration for this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ibigbug I understand the use case you are trying to solve. My question was for the use case where a user might have set the env var for http_proxy, but not in settings. What happens then, with your code changes?

I tested your code in 2 scenarios in Windows, this is what I see for the env that gets passed to the child process running the go get

  1. When http_proxy is set in env var but not in VS Code settings
    image

  2. When different values are set for http_proxy in env var and VS Code settings
    image

You will have to do 2 changes I believe

  1. Use upper case while setting the http_proxy variable
  2. If there is no VS Code setting for http_proxy, then do not change the process.env

missing.reduce((res: Promise<string[]>, tool: string) => {
return res.then(sofar => new Promise<string[]>((resolve, reject) => {
cp.execFile(goRuntimePath, ['get', '-u', '-v', tools[tool]], { env: process.env }, (err, stdout, stderr) => {
cp.execFile(goRuntimePath, ['get', '-u', '-v', tools[tool]], { env: Object.assign({}, process.env, { http_proxy, https_proxy }) }, (err, stdout, stderr) => {
if (err) {
outputChannel.appendLine('Installing ' + tool + ' FAILED');
let failureReason = tool + ';;' + err + stdout.toString() + stderr.toString();
Expand Down