-
Notifications
You must be signed in to change notification settings - Fork 4
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
Node Explorer: prompt to add nodes to SSH config file for VSCode remotes list #233
Conversation
…tes list Signed-off-by: Naman Sood <mail@nsood.in>
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.
Looks great - just a few small suggestions. I think we could further improve the UX by removing the command and always syncing the SSH config when tailscale.node.openRemoteCode
is run. It's the only thing that needs it, and in almost all situations it will be required.
One thing we have to figure out is how someone would change their selection if they choose to use a different SSH user. One thought is to have a context menu item that is only displayed if that setting is used - we could accomplish that with pattern matching on the context. So, in the event they choose to use the SSH config user, they would have a context button for "Update SSH config user" (wording could be improved) |
My thoughts there were that once the user decouples their Tailscale SSH username from the one in the SSH config file, the one in the config file is no longer our business - we use the username in the Tailscale settings to connect anyway. |
The reason I added the command was that if you decide not to persist your changes to the SSH config files initially, there is otherwise no way to walk back that decision. Unless we'd prefer not to remember that and prompt the user every time? |
Signed-off-by: Naman Sood <mail@nsood.in>
src/node-explorer-provider.ts
Outdated
@@ -620,6 +622,10 @@ export class NodeExplorerProvider | |||
async (node: PeerRoot | FileExplorer) => { | |||
const { addr, path } = extractAddrAndPath(node); | |||
|
|||
if (addr && this.configManager.config.hosts?.[addr].persistToSSHConfig !== false) { | |||
syncSSHConfig(addr, this.configManager); |
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.
Lets await this
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.
Just one small change, then LGTM! This is a huge improvement for the user experience when using open in remote. Great work.
Signed-off-by: Naman Sood <mail@nsood.in>
``` % git log --pretty=oneline v0.6.2..release-branch/v0.6 (HEAD -> release-branch/v0.6) chore(deps): update dependency @vscode/vsce to ^2.21.0 (#223) chore(deps): update dependency @types/vscode-webview to ^1.57.2 (#218) chore(deps): update dependency concurrently to ^8.2.1 (#219) chore(deps): update dependency lint-staged to ^13.3.0 (#196) tsrelay: update for 1.50 (#239) Node Explorer: prompt to add nodes to SSH config file for VSCode remotes list (#233) Node Explorer: accept subdirectories of ~ for root directory (#230) File Explorer: add configuration option to hide dotfiles (#221) package.json: Remove from testing category (#226) chore(deps): update dependency swr to ^2.2.2 (#195) chore(deps): update dependency @types/react to ^18.2.21 (#194) chore(deps): update eslint (#137) package.json: Updates the description (#215) README.md: Use lowercase "internet" (#216) ``` --------- Signed-off-by: Tyler Smalley <tyler@tailscale.com> Co-authored-by: Naman Sood <mail@nsood.in>
``` % git log --pretty=oneline v0.6.2..release-branch/v0.6 (HEAD -> release-branch/v0.6) chore(deps): update dependency @vscode/vsce to ^2.21.0 (#223) chore(deps): update dependency @types/vscode-webview to ^1.57.2 (#218) chore(deps): update dependency concurrently to ^8.2.1 (#219) chore(deps): update dependency lint-staged to ^13.3.0 (#196) tsrelay: update for 1.50 (#239) Node Explorer: prompt to add nodes to SSH config file for VSCode remotes list (#233) Node Explorer: accept subdirectories of ~ for root directory (#230) File Explorer: add configuration option to hide dotfiles (#221) package.json: Remove from testing category (#226) chore(deps): update dependency swr to ^2.2.2 (#195) chore(deps): update dependency @types/react to ^18.2.21 (#194) chore(deps): update eslint (#137) package.json: Updates the description (#215) README.md: Use lowercase "internet" (#216) ``` --------- Signed-off-by: Tyler Smalley <tyler@tailscale.com> Co-authored-by: Naman Sood <mail@nsood.in>
Fixes #217.