Edit database connections #8
Conversation
…the user select af connection from a quick pick list
…hed correctly when retrieveing the value for editing
…et the user pick a connection from a quick pick menu
src/common/utility.ts
Outdated
| connectionNode.editConnection(context, mysqlTreeDataProvider); | ||
| } else { | ||
| // No connection has been selected, let the user pick one | ||
| const connections = context.globalState.get<{ [key: string]: IConnection }>(Constants.GlobalStateMySQLConectionsKey); |
There was a problem hiding this comment.
Pick connection logic is similar in deleteConnection and editConnection. Please merge them
src/model/connectionNode.ts
Outdated
| public async editConnection(context?: vscode.ExtensionContext, mysqlTreeDataProvider?: MySQLTreeDataProvider) { | ||
| AppInsightsClient.sendEvent("editConncetion"); | ||
|
|
||
| const host = await vscode.window.showInputBox({ prompt: "The hostname of the database", placeHolder: "host", ignoreFocusOut: true, value: this.host }); |
There was a problem hiding this comment.
This logic is similar to addConnection. Please meege them
src/model/connectionNode.ts
Outdated
| mysqlTreeDataProvider.refresh(); | ||
| } | ||
|
|
||
| public async editConnection(context?: vscode.ExtensionContext, mysqlTreeDataProvider?: MySQLTreeDataProvider) { |
src/common/utility.ts
Outdated
| const selectedConnection = connections[selectedConnectionId]; | ||
|
|
||
| if (selectedConnection !== undefined) { | ||
| const dummyNode = new ConnectionNode(selectedConnectionId, selectedConnection.host, selectedConnection.user, selectedConnection.password, |
There was a problem hiding this comment.
Is it good practice to do this?
There was a problem hiding this comment.
Maybe not.. but should we move the edit/delete methods away from the connectionNode and onto the connection object instead?
We could change the IConnection to an abstract class instead, and implement the methods in here instead?
|
@formulahendry Can you look into merging the changes? I also added the ability to "Drop table" from the Tablenode |
| "group": "mysql@1" | ||
| }, | ||
| { | ||
| "command": "mysql.editConnection", |
There was a problem hiding this comment.
Put this before mysql.deleteConnection
| return; | ||
| } | ||
|
|
||
| AppInsightsClient.sendEvent("drop table"); |
| }; | ||
| Global.activeConnection = connection; | ||
|
|
||
| Utility.runQuery(sql, connection); |
There was a problem hiding this comment.
How about refreshing the table list after deletion is successful?
There was a problem hiding this comment.
How do i refresh the databaseNode from inside the tableNode class? I want to call:
mysqlTreeDataProvider.refresh(databaseNode);
How can i get the "parent" databaseNode for a tableNode?
There was a problem hiding this comment.
You could pass databaseNode to constructor of tableNode
There was a problem hiding this comment.
Alright ill look into that
| }; | ||
| const editConnection = await Utility.createConnectionFromInput(context, copyFrom); | ||
|
|
||
| if (editConnection !== undefined) { |
There was a problem hiding this comment.
This logic is similar to addConnection. Please help merge them.
There was a problem hiding this comment.
Merge them into a addOrEditConnection method?
Should the metod(s) be placed on the connectionNode or the mysqlTreeDataProvider class?
There was a problem hiding this comment.
We could wrap these similar code snippet into a method into https://github.com/formulahendry/vscode-mysql/blob/master/src/common/utility.ts
There was a problem hiding this comment.
(We still keep editConnection in connectionNode and addConnection in mysqlTreeDataProvider, and call shared method in utility)
| } | ||
| } | ||
|
|
||
| await vscode.window.showQuickPick(items).then(async (selection) => { |
There was a problem hiding this comment.
Better to use: const selection = await vscode.window.showQuickPick(items)
| await vscode.window.showQuickPick(items).then(async (selection) => { | ||
| // the user canceled the selection | ||
| if (!selection) { | ||
| return selectedConnection; |
| return selectedConnection; | ||
| } | ||
|
|
||
| const selectedConnectionId = Object.keys(connections)[items.indexOf(selection)]; |
There was a problem hiding this comment.
What if there are two connections have the same host and user?
There was a problem hiding this comment.
You would still get the correct selection from the pick menu.. but you wont be able to differ from the two when the menu is shown.. how would you enable anyone to see the difference between two connections with the same host and user? showing the internal id of the connection wont be to much use for the user.
There was a problem hiding this comment.
If we could still get the correct selection from the pick menu, then it will be OK. No extra change needed.
2 participants
You can now edit a database connection, regardless if you right click it in the Tree or run the command "Edit Connection". If you run the command from the Command Palette, the user will have to select which MySQL connection he wants to edit. The same has been added for the "Delete Connection" command.