-
Notifications
You must be signed in to change notification settings - Fork 48
initial support for server's settings from intersystems.servers #172
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
Conversation
@daimor do you want this approved and merged now? |
Work in progress, it was happened to be a bit more complicated, than I thought. |
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.
Some cosmetic changes. I haven't yet tested the VSIX, but am confident you will have done that.
package.json
Outdated
}, | ||
"server": { | ||
"type": "string", | ||
"pattern": "^[A-Za-z0-9-._~]+$", |
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 think we no longer allow uppercase characters in server names.
package.json
Outdated
"server": { | ||
"type": "string", | ||
"pattern": "^[A-Za-z0-9-._~]+$", | ||
"markdownDescription": "Server defined in `intersystems.servers`" |
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.
Make this
`#intersystems.servers#`
and it will hyperlink to that part of Settings.
src/commands/serverActions.ts
Outdated
if (workspaceState.get(workspaceFolder + ":docker", true)) { | ||
terminal.push({ | ||
if (workspaceState.get(workspaceFolder + ":docker", false)) { | ||
actions.push({ | ||
id: "openDockerTerminal", | ||
label: "Open terminal in docker", |
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.
"Open Terminal in Docker"
Co-authored-by: John Murray <[email protected]>
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.
Final tiny nitpick.
package.json
Outdated
@@ -558,29 +558,29 @@ | |||
"description": "Password of username. If not set here it must be provided when connecting." | |||
}, | |||
"https": { | |||
"description": "Use SSL to access to API", | |||
"description": "Use SSL to access to API.", |
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.
"description": "Use SSL to access to API.", | |
"description": "Use SSL/TLS to access the server.", |
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.
LGTM
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.
package.json needs this adding so that when this extension gets installed it will pull in the Server Manager one.
"extensionDependencies": [
"intersystems-community.servermanager"
],
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.
Thanks
No description provided.