From 5770d5931076e8e4ad149be3ccf8899cc64b7643 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Sat, 6 Mar 2021 12:23:28 -0500 Subject: [PATCH] Fixes #1411 - stops click edit on compare node Adds explicit edit action when there is a comparison --- package.json | 34 +++++++++++++------- src/views/nodes/compareBranchNode.ts | 21 ++++++++----- src/views/nodes/viewNode.ts | 25 ++++++++------- src/views/searchAndCompareView.ts | 7 ----- src/views/viewCommands.ts | 61 +++++++++++++++++------------------- 5 files changed, 78 insertions(+), 70 deletions(-) diff --git a/package.json b/package.json index 8bada8f..1d9162d 100644 --- a/package.json +++ b/package.json @@ -4257,6 +4257,12 @@ "icon": "$(close)" }, { + "command": "gitlens.views.editNode", + "title": "Edit...", + "category": "GitLens", + "icon": "$(edit)" + }, + { "command": "gitlens.views.expandNode", "title": "Expand", "category": "GitLens" @@ -4852,12 +4858,6 @@ "icon": "$(search)" }, { - "command": "gitlens.views.searchAndCompare.edit", - "title": "Edit...", - "category": "GitLens", - "icon": "$(edit)" - }, - { "command": "gitlens.views.searchAndCompare.selectForCompare", "title": "Compare References...", "category": "GitLens", @@ -5809,6 +5809,10 @@ "when": "false" }, { + "command": "gitlens.views.editNode", + "when": "false" + }, + { "command": "gitlens.views.expandNode", "when": "false" }, @@ -6233,10 +6237,6 @@ "when": "false" }, { - "command": "gitlens.views.searchAndCompare.edit", - "when": "false" - }, - { "command": "gitlens.views.searchAndCompare.selectForCompare", "when": "false" }, @@ -8111,6 +8111,11 @@ "group": "inline@99" }, { + "command": "gitlens.views.editNode", + "when": "viewItem =~ /gitlens:compare:branch\\b(?=.*?\\b\\+comparing\\b)/", + "group": "inline@98" + }, + { "command": "gitlens.views.setBranchComparisonToWorking", "when": "viewItem =~ /gitlens:compare:branch\\b(?=.*?\\b\\+root\\b)(?=.*?\\b\\+current\\b)(?=.*?\\b\\+branch\\b)/", "group": "inline@2" @@ -8121,6 +8126,11 @@ "group": "inline@2" }, { + "command": "gitlens.views.editNode", + "when": "viewItem =~ /gitlens:compare:branch\\b(?=.*?\\b\\+comparing\\b)/", + "group": "1_gitlens@1" + }, + { "command": "gitlens.views.setBranchComparisonToWorking", "when": "viewItem =~ /gitlens:compare:branch\\b(?=.*?\\b\\+root\\b)(?=.*?\\b\\+current\\b)(?=.*?\\b\\+branch\\b)/", "group": "1_gitlens@2" @@ -8201,12 +8211,12 @@ "group": "8_gitlens_actions@1" }, { - "command": "gitlens.views.searchAndCompare.edit", + "command": "gitlens.views.editNode", "when": "viewItem =~ /gitlens:search:results(?!:)\\b/", "group": "inline@1" }, { - "command": "gitlens.views.searchAndCompare.edit", + "command": "gitlens.views.editNode", "when": "viewItem =~ /gitlens:search:results(?!:)\\b/", "group": "1_gitlens_actions@1" }, diff --git a/src/views/nodes/compareBranchNode.ts b/src/views/nodes/compareBranchNode.ts index 602d6fd..1561974 100644 --- a/src/views/nodes/compareBranchNode.ts +++ b/src/views/nodes/compareBranchNode.ts @@ -154,13 +154,15 @@ export class CompareBranchNode extends ViewNode this.compareWith()], - }; + if (this._compareWith == null) { + item.command = { + title: `Compare ${this.branch.name}${this.compareWithWorkingTree ? ' (working)' : ''} with${ + GlyphChars.Ellipsis + }`, + command: 'gitlens.views.editNode', + arguments: [this], + }; + } item.contextValue = `${ContextValues.CompareBranch}${this.branch.current ? '+current' : ''}+${ this.comparisonType }${this._compareWith == null ? '' : '+comparing'}${this.root ? '+root' : ''}`; @@ -185,6 +187,11 @@ export class CompareBranchNode extends ViewNode extends ViewRef } } -export function nodeSupportsClearing(node: ViewNode): node is ViewNode & { clear(): void | Promise } { - return typeof (node as ViewNode & { clear(): void | Promise }).clear === 'function'; -} - export interface PageableViewNode { readonly id: string; limit?: number; @@ -188,7 +184,7 @@ export abstract class SubscribeableViewNode extends V this.view.onDidChangeNodeCollapsibleState(this.onNodeCollapsibleStateChanged, this), ]; - if (viewSupportsAutoRefresh(this.view)) { + if (canAutoRefreshView(this.view)) { disposables.push(this.view.onDidChangeAutoRefresh(this.onAutoRefreshChanged, this)); } @@ -290,11 +286,7 @@ export abstract class SubscribeableViewNode extends V @debug() async ensureSubscription() { // We only need to subscribe if we are visible and if auto-refresh enabled (when supported) - if ( - !this.canSubscribe || - !this.view.visible || - (viewSupportsAutoRefresh(this.view) && !this.view.autoRefresh) - ) { + if (!this.canSubscribe || !this.view.visible || (canAutoRefreshView(this.view) && !this.view.autoRefresh)) { await this.unsubscribe(); return; @@ -438,10 +430,19 @@ interface AutoRefreshableView { autoRefresh: boolean; onDidChangeAutoRefresh: Event; } -export function viewSupportsAutoRefresh(view: View): view is View & AutoRefreshableView { + +export function canAutoRefreshView(view: View): view is View & AutoRefreshableView { return Functions.is(view, 'onDidChangeAutoRefresh'); } -export function viewSupportsNodeDismissal(view: View): view is View & { dismissNode(node: ViewNode): void } { +export function canClearNode(node: ViewNode): node is ViewNode & { clear(): void | Promise } { + return typeof (node as ViewNode & { clear(): void | Promise }).clear === 'function'; +} + +export function canEditNode(node: ViewNode): node is ViewNode & { edit(): void | Promise } { + return typeof (node as ViewNode & { edit(): void | Promise }).edit === 'function'; +} + +export function canViewDismissNode(view: View): view is View & { dismissNode(node: ViewNode): void } { return typeof (view as View & { dismissNode(node: ViewNode): void }).dismissNode === 'function'; } diff --git a/src/views/searchAndCompareView.ts b/src/views/searchAndCompareView.ts index e3c636a..9e61dc3 100644 --- a/src/views/searchAndCompareView.ts +++ b/src/views/searchAndCompareView.ts @@ -293,7 +293,6 @@ export class SearchAndCompareView extends ViewBase this.reveal(results, options)); } - private edit(node: SearchResultsNode) { - if (!(node instanceof SearchResultsNode)) return undefined; - - return node.edit(); - } - private setFilesLayout(layout: ViewFilesLayout) { return configuration.updateEffective('views', this.configKey, 'files', 'layout', layout); } diff --git a/src/views/viewCommands.ts b/src/views/viewCommands.ts index 4eef9c1..da5ce35 100644 --- a/src/views/viewCommands.ts +++ b/src/views/viewCommands.ts @@ -21,6 +21,9 @@ import { BranchesNode, BranchNode, BranchTrackingStatusNode, + canClearNode, + canEditNode, + canViewDismissNode, CommitFileNode, CommitNode, CompareBranchNode, @@ -31,7 +34,6 @@ import { FolderNode, LineHistoryNode, MergeConflictFileNode, - nodeSupportsClearing, PageableViewNode, PagerNode, PullRequestNode, @@ -49,7 +51,6 @@ import { ViewNode, ViewRefFileNode, ViewRefNode, - viewSupportsNodeDismissal, } from './nodes'; import { debug } from '../system'; import { runGitCommandInTerminal } from '../terminal'; @@ -63,6 +64,7 @@ interface CompareSelectedInfo { export class ViewCommands { constructor() { + commands.registerCommand('gitlens.views.clearNode', (n: ViewNode) => canClearNode(n) && n.clear(), this); commands.registerCommand( 'gitlens.views.copy', async (selection: ViewNode | ViewNode[]) => { @@ -78,35 +80,30 @@ export class ViewCommands { this, ); commands.registerCommand( - 'gitlens.views.refreshNode', - (node: ViewNode, reset?: boolean) => { - if (reset == null && PageableViewNode.is(node)) { - node.limit = undefined; - node.view.resetNodeLastKnownLimit(node); - } - - return node.view.refreshNode(node, reset == null ? true : reset); - }, + 'gitlens.views.dismissNode', + (n: ViewNode) => canViewDismissNode(n.view) && n.view.dismissNode(n), this, ); + commands.registerCommand('gitlens.views.editNode', (n: ViewNode) => canEditNode(n) && n.edit(), this); commands.registerCommand( 'gitlens.views.expandNode', - (node: ViewNode) => node.view.reveal(node, { select: false, focus: false, expand: 3 }), + (n: ViewNode) => n.view.reveal(n, { select: false, focus: false, expand: 3 }), this, ); + commands.registerCommand('gitlens.views.loadMoreChildren', (n: PagerNode) => n.loadMore(), this); + commands.registerCommand('gitlens.views.loadAllChildren', (n: PagerNode) => n.loadAll(), this); commands.registerCommand( - 'gitlens.views.clearNode', - (node: ViewNode) => nodeSupportsClearing(node) && node.clear(), - this, - ); - commands.registerCommand( - 'gitlens.views.dismissNode', - (node: ViewNode) => viewSupportsNodeDismissal(node.view) && node.view.dismissNode(node), + 'gitlens.views.refreshNode', + (n: ViewNode, reset?: boolean) => { + if (reset == null && PageableViewNode.is(n)) { + n.limit = undefined; + n.view.resetNodeLastKnownLimit(n); + } + + return n.view.refreshNode(n, reset == null ? true : reset); + }, this, ); - commands.registerCommand('gitlens.views.executeNodeCallback', (fn: () => Promise) => fn(), this); - commands.registerCommand('gitlens.views.loadMoreChildren', (node: PagerNode) => node.loadMore(), this); - commands.registerCommand('gitlens.views.loadAllChildren', (node: PagerNode) => node.loadAll(), this); commands.registerCommand( 'gitlens.views.setShowRelativeDateMarkersOn', @@ -259,6 +256,16 @@ export class ViewCommands { } @debug() + private browseRepoAtRevision(node: ViewRefNode, options?: { before?: boolean; openInNewWindow?: boolean }) { + if (!(node instanceof ViewRefNode)) return Promise.resolve(); + + return GitActions.browseAtRevision(node.uri, { + before: options?.before, + openInNewWindow: options?.openInNewWindow, + }); + } + + @debug() private cherryPick(node: CommitNode) { if (!(node instanceof CommitNode)) return Promise.resolve(); @@ -337,16 +344,6 @@ export class ViewCommands { } @debug() - private browseRepoAtRevision(node: ViewRefNode, options?: { before?: boolean; openInNewWindow?: boolean }) { - if (!(node instanceof ViewRefNode)) return Promise.resolve(); - - return GitActions.browseAtRevision(node.uri, { - before: options?.before, - openInNewWindow: options?.openInNewWindow, - }); - } - - @debug() private fetch(node: RemoteNode | RepositoryNode | BranchNode | BranchTrackingStatusNode) { if (node instanceof RepositoryNode) return GitActions.fetch(node.repo); if (node instanceof RemoteNode) return GitActions.Remote.fetch(node.remote.repoPath, node.remote.name);