-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(vscode): add proxy support for vscode #2921
Conversation
eryue0220
commented
Aug 20, 2024
- This pr is to solve Support http proxy config in VSCode settings #2893
proxy?: { | ||
authorization: string; | ||
url: string; | ||
noProxy: string | string[]; |
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.
noProxy: string | string[]; |
I recommend removing this option for convenient of implementation in first step
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.
Updated.
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.
Otherwise LGTM
@@ -12,6 +12,7 @@ export type AgentConfig = { | |||
requestTimeout: number; | |||
}; | |||
proxy: { | |||
proxySupport: "on" | "off"; |
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.
proxySupport: "on" | "off"; | |
enabled: boolean |
No need to reuse the variable name from vscode workspace, as AgentConfig is IDE / Extension agnostic.
Please also rebase against main branch |
const fieldsToCheck = [ | ||
{ | ||
key: "server.endpoint", | ||
validation: (key: string, x: string) => { |
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.
validation: (key: string, x: string) => { | |
validation: (key: keyof ClientProvidedConfig, x: string) => { |
Just use explict type?
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.
Updated.
}, | ||
{ | ||
key: "server.token", | ||
validation: (key: string, x: string) => { |
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.
ditto, and the function can be reused?
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.
Right, I just struggled with type and ignored.
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.
Otherwise LGTM
@@ -12,6 +12,8 @@ export type AgentConfig = { | |||
requestTimeout: number; | |||
}; | |||
proxy: { | |||
enabled: boolean; |
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.
I prefer to not add proxy.enabled
. You can set the proxy.url
to empty to disable the proxy.
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.
Updated.
* feat: add proxy support for vscode * feat: complete proxy feature * feat: add proxy support for vscode * fix: remove noProxy configuration * fix: update configuration strategy * fix: support vscode proxy update * [autofix.ci] apply automated fixes * fix: cr * feat: reactor init function * [autofix.ci] apply automated fixes * feat: refactor configuration init & update * fix: format * fix: cr * fix: cr * fix: cr bug --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>