Adopt vscode.diff and vscode.open for tree commands - #2334
Conversation
Start with the GitFileChangeNode Part of #2333
|
Rachel Macfarlane (@RMacfarlane) I just picked a place to start. I figured it would be easier if we just to one or two uses of |
|
I'm a bit confused about these changes overall, previously the commands for the tree items eventually called |
|
Yes, using |
|
Even after approval, I won't merge this change until insiders is updated, since it requires changes in the core to add the |
|
|
||
| async openDiff(folderManager: FolderRepositoryManager): Promise<void> { | ||
| const command = await this.openDiffCommand(folderManager); | ||
| vscode.commands.executeCommand(command.command, command.arguments); |
There was a problem hiding this comment.
I think we need to spread the arguments here with ...command.arguments!, otherwise it seems like vscode interprets the whole array as the first argument
| } | ||
|
|
||
| /** | ||
| * File change node whose content can not be resolved locally and we direct users to GitHub. |
There was a problem hiding this comment.
This is an existing issue, but the RemoteFileChangeNode also uses pr.openDiffView as its command but doesn't have a openDiff method
There was a problem hiding this comment.
package-lock.json on auchenberg/pullrequest-demo#105 is an example. It's a big file, on github.com when you view the file changes it shows a "Load diff" button instead of the file content. At first for these types of changes we just showed a notification asking if you wanted to view it on GitHub, but I believe I made changes to the content provider to go make the additional call for this case. So I think just making the same vscode.diff call would work?
There was a problem hiding this comment.
I think just making the same vscode.diff call would work. Not sure how that command handles large files.
| } | ||
|
|
||
| async resolve(): Promise<void> { | ||
| console.log('resolving'); |
There was a problem hiding this comment.
extra console :)
| } | ||
| } | ||
|
|
||
| async resolveTreeItem?(item: vscode.TreeItem, element: TreeNode): Promise<vscode.TreeItem> { |
There was a problem hiding this comment.
didn't know about this API, cool!
There was a problem hiding this comment.
actually, how often is this called? I'm assuming just once, and then the command would get filled in then? I'm wondering if we will need to refresh the tree or something if the user changes the openDiffOnClick setting.
There was a problem hiding this comment.
It's new! Good catch, yes, we should refresh the tree if that setting is changed.
| return vscode.commands.executeCommand('review.openFile', value); | ||
| } | ||
| const command = value instanceof GitFileChangeNode ? value.openFileCommand() : openFileCommand(value); | ||
| vscode.commands.executeCommand(command.title, command.arguments); |
There was a problem hiding this comment.
typo here, should be command.command, and also ... on arguments
There was a problem hiding this comment.
Thanks for catching!
2 participants
Start with the GitFileChangeNode
Part of #2333