From abba69b9b13ba47a7b494a2f20379f7070894138 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 8 Sep 2023 12:25:05 -0400 Subject: [PATCH] Always persists items in search & compare view - Removes pinning notion from search & compare view - Removes "Keep Results" toggle from search & compare view Persists "reviewed" checkboxes in comparisons --- package.json | 74 +--------------- src/constants.ts | 26 +++--- src/views/nodes/compareBranchNode.ts | 67 +++++++++++--- src/views/nodes/comparePickerNode.ts | 5 -- src/views/nodes/compareResultsNode.ts | 160 +++++++++++++++++++++------------- src/views/nodes/resultsCommitsNode.ts | 2 +- src/views/nodes/resultsFileNode.ts | 9 +- src/views/nodes/searchResultsNode.ts | 118 +++++++++++-------------- src/views/nodes/viewNode.ts | 37 ++++---- src/views/searchAndCompareView.ts | 158 ++++++++++----------------------- src/views/viewBase.ts | 49 +++++++++-- 11 files changed, 331 insertions(+), 374 deletions(-) diff --git a/package.json b/package.json index 833c3d9..19f1838 100644 --- a/package.json +++ b/package.json @@ -6869,18 +6869,6 @@ "category": "GitLens" }, { - "command": "gitlens.views.searchAndCompare.pin", - "title": "Pin", - "category": "GitLens", - "icon": "$(pin)" - }, - { - "command": "gitlens.views.searchAndCompare.unpin", - "title": "Unpin", - "category": "GitLens", - "icon": "$(pinned)" - }, - { "command": "gitlens.views.searchAndCompare.refresh", "title": "Refresh", "category": "GitLens", @@ -6917,18 +6905,6 @@ "icon": "$(list-flat)" }, { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOn", - "title": "Keep Results", - "category": "GitLens", - "icon": "$(unlock)" - }, - { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOff", - "title": "Keep Results", - "category": "GitLens", - "icon": "$(lock)" - }, - { "command": "gitlens.views.searchAndCompare.setShowAvatarsOn", "title": "Show Avatars", "category": "GitLens" @@ -9556,14 +9532,6 @@ "when": "false" }, { - "command": "gitlens.views.searchAndCompare.pin", - "when": "false" - }, - { - "command": "gitlens.views.searchAndCompare.unpin", - "when": "false" - }, - { "command": "gitlens.views.searchAndCompare.refresh", "when": "false" }, @@ -9588,14 +9556,6 @@ "when": "false" }, { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOn", - "when": "false" - }, - { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOff", - "when": "false" - }, - { "command": "gitlens.views.searchAndCompare.setShowAvatarsOn", "when": "false" }, @@ -11067,16 +11027,6 @@ "group": "navigation@10" }, { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOn", - "when": "view =~ /^gitlens\\.views\\.searchAndCompare\\b/ && !gitlens:views:searchAndCompare:keepResults", - "group": "navigation@12" - }, - { - "command": "gitlens.views.searchAndCompare.setKeepResultsToOff", - "when": "view =~ /^gitlens\\.views\\.searchAndCompare\\b/ && gitlens:views:searchAndCompare:keepResults", - "group": "navigation@13" - }, - { "command": "gitlens.views.searchAndCompare.clear", "when": "view =~ /^gitlens\\.views\\.searchAndCompare\\b/", "group": "navigation@98" @@ -12483,7 +12433,7 @@ }, { "command": "gitlens.views.dismissNode", - "when": "viewItem =~ /gitlens:(compare:picker|(compare|search):results(?!:)\\b(?!.*?\\b\\+pinned\\b))\\b(?!:(commits|files))/", + "when": "viewItem =~ /gitlens:(compare:picker|(compare|search):results(?!:)\\b)\\b(?!:(commits|files))/", "group": "inline@99" }, { @@ -12572,16 +12522,6 @@ "group": "inline@97" }, { - "command": "gitlens.views.searchAndCompare.pin", - "when": "viewItem =~ /gitlens:(compare|search):results(?!:)\\b(?!.*?\\b\\+pinned\\b)/", - "group": "inline@98" - }, - { - "command": "gitlens.views.searchAndCompare.unpin", - "when": "viewItem =~ /gitlens:(compare|search):results(?!:)\\b(?=.*?\\b\\+pinned\\b)/", - "group": "inline@98" - }, - { "command": "gitlens.views.searchAndCompare.swapComparison", "when": "viewItem =~ /gitlens:compare:results(?!:)\\b(?!.*?\\b\\+working\\b)/", "group": "1_gitlens_actions@2" @@ -12592,16 +12532,6 @@ "group": "2_gitlens_quickopen@1" }, { - "command": "gitlens.views.searchAndCompare.pin", - "when": "viewItem =~ /gitlens:(compare|search):results(?!:)\\b(?!.*?\\b\\+pinned\\b)/", - "group": "8_gitlens_actions@1" - }, - { - "command": "gitlens.views.searchAndCompare.unpin", - "when": "viewItem =~ /gitlens:(compare|search):results(?!:)\\b(?=.*?\\b\\+pinned\\b)/", - "group": "8_gitlens_actions@1" - }, - { "command": "gitlens.views.editNode", "when": "viewItem =~ /gitlens:search:results(?!:)\\b/", "group": "inline@1" @@ -12787,7 +12717,7 @@ }, { "command": "gitlens.views.dismissNode", - "when": "viewItem =~ /gitlens:(compare:picker:ref|(compare|search):results(?!:)\\b(?!.*?\\b\\+pinned\\b))\\b(?!:(commits|files))/", + "when": "viewItem =~ /gitlens:(compare:picker:ref|(compare|search):results(?!:)\\b)\\b(?!:(commits|files))/", "group": "8_gitlens_actions@98" }, { diff --git a/src/constants.ts b/src/constants.ts index 4e8c160..7f0b765 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -536,7 +536,6 @@ export type ContextKeys = | `${typeof extensionPrefix}:views:fileHistory:editorFollowing` | `${typeof extensionPrefix}:views:lineHistory:editorFollowing` | `${typeof extensionPrefix}:views:repositories:autoRefresh` - | `${typeof extensionPrefix}:views:searchAndCompare:keepResults` | `${typeof extensionPrefix}:vsls` | `${typeof extensionPrefix}:plus` | `${typeof extensionPrefix}:plus:disallowedRepos` @@ -769,8 +768,8 @@ export type DeprecatedWorkspaceStorage = { 'graph:banners:dismissed': Record; /** @deprecated use `graph:filtersByRepo.excludeRefs` */ 'graph:hiddenRefs': Record; - /** @deprecated use `views:searchAndCompare:pinned` */ - 'pinned:comparisons': Record; + /** @deprecated */ + 'views:searchAndCompare:keepResults': boolean; }; export type WorkspaceStorage = { @@ -784,8 +783,7 @@ export type WorkspaceStorage = { 'starred:branches': StoredStarred; 'starred:repositories': StoredStarred; 'views:repositories:autoRefresh': boolean; - 'views:searchAndCompare:keepResults': boolean; - 'views:searchAndCompare:pinned': StoredPinnedItems; + 'views:searchAndCompare:pinned': StoredSearchAndCompareItems; 'views:commitDetails:autolinksExpanded': boolean; } & { [key in `confirm:ai:tos:${AIProviders}`]: boolean } & { [key in `connected:${string}`]: boolean }; @@ -811,6 +809,7 @@ export interface StoredBranchComparison { ref: string; notation: '..' | '...' | undefined; type: Exclude | undefined; + checkedFiles?: string[]; } export type StoredBranchComparisons = Record; @@ -853,16 +852,18 @@ export interface StoredNamedRef { ref: string; } -export interface StoredPinnedComparison { +export interface StoredComparison { type: 'comparison'; timestamp: number; path: string; ref1: StoredNamedRef; ref2: StoredNamedRef; notation?: '..' | '...'; + + checkedFiles?: string[]; } -export interface StoredPinnedSearch { +export interface StoredSearch { type: 'search'; timestamp: number; path: string; @@ -878,14 +879,7 @@ export interface StoredPinnedSearch { search: StoredSearchQuery; } -export type StoredPinnedItem = StoredPinnedComparison | StoredPinnedSearch; -export type StoredPinnedItems = Record; +export type StoredSearchAndCompareItem = StoredComparison | StoredSearch; +export type StoredSearchAndCompareItems = Record; export type StoredStarred = Record; export type RecentUsage = Record; - -interface DeprecatedPinnedComparison { - path: string; - ref1: StoredNamedRef; - ref2: StoredNamedRef; - notation?: '..' | '...'; -} diff --git a/src/views/nodes/compareBranchNode.ts b/src/views/nodes/compareBranchNode.ts index fcd9f8b..ee89fb8 100644 --- a/src/views/nodes/compareBranchNode.ts +++ b/src/views/nodes/compareBranchNode.ts @@ -1,3 +1,4 @@ +import type { Disposable, TreeCheckboxChangeEvent } from 'vscode'; import { ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode'; import { ViewShowBranchComparison } from '../../config'; import type { StoredBranchComparison, StoredBranchComparisons } from '../../constants'; @@ -13,13 +14,20 @@ import { getSettledValue } from '../../system/promise'; import { pluralize } from '../../system/string'; import type { ViewsWithBranches } from '../viewBase'; import type { WorktreesView } from '../worktreesView'; +import { + getComparisonCheckedFiles, + getComparisonStoragePrefix, + resetComparisonCheckedFiles, + restoreComparisonCheckedFiles, +} from './compareResultsNode'; import type { CommitsQueryResults } from './resultsCommitsNode'; import { ResultsCommitsNode } from './resultsCommitsNode'; import type { FilesQueryResults } from './resultsFilesNode'; import { ResultsFilesNode } from './resultsFilesNode'; -import { ContextValues, getViewNodeId, ViewNode } from './viewNode'; +import type { ViewNode } from './viewNode'; +import { ContextValues, getViewNodeId, SubscribeableViewNode } from './viewNode'; -export class CompareBranchNode extends ViewNode { +export class CompareBranchNode extends SubscribeableViewNode { private _children: ViewNode[] | undefined; private _compareWith: StoredBranchComparison | undefined; @@ -34,11 +42,15 @@ export class CompareBranchNode extends ViewNode | undefined { + return this.view.onDidChangeNodesCheckedState(this.onNodesCheckedStateChanged, this); + } + + private onNodesCheckedStateChanged(e: TreeCheckboxChangeEvent) { + const prefix = getComparisonStoragePrefix(this.getStorageId()); + if (e.items.some(([n]) => n.id?.startsWith(prefix))) { + void this.storeCompareWith(false); + } + } + async getChildren(): Promise { if (this._compareWith == null) return []; @@ -198,7 +221,7 @@ export class CompareBranchNode extends ViewNode) { if (this._compareWith != null) { - await this.updateCompareWith({ ...this._compareWith, type: comparisonType }); + await this.updateCompareWith({ ...this._compareWith, type: comparisonType, checkedFiles: undefined }); } else { this.showComparison = comparisonType; } @@ -355,11 +378,15 @@ export class CompareBranchNode extends ViewNode { readonly order: number = Date.now(); - readonly pinned: boolean = false; constructor( view: SearchAndCompareView, @@ -23,10 +22,6 @@ export class ComparePickerNode extends ViewNode { super(unknownGitUri, view, parent); } - get canDismiss(): boolean { - return true; - } - getChildren(): ViewNode[] { return []; } diff --git a/src/views/nodes/compareResultsNode.ts b/src/views/nodes/compareResultsNode.ts index 7d2d915..77c2759 100644 --- a/src/views/nodes/compareResultsNode.ts +++ b/src/views/nodes/compareResultsNode.ts @@ -1,4 +1,5 @@ -import { ThemeIcon, TreeItem, TreeItemCollapsibleState, window } from 'vscode'; +import type { Disposable, TreeCheckboxChangeEvent } from 'vscode'; +import { ThemeIcon, TreeItem, TreeItemCheckboxState, TreeItemCollapsibleState, window } from 'vscode'; import { md5 } from '@env/crypto'; import type { StoredNamedRef } from '../../constants'; import { GitUri } from '../../git/gitUri'; @@ -8,19 +9,17 @@ import { debug, log } from '../../system/decorators/log'; import { getSettledValue } from '../../system/promise'; import { pluralize } from '../../system/string'; import type { SearchAndCompareView } from '../searchAndCompareView'; +import type { View } from '../viewBase'; import type { CommitsQueryResults } from './resultsCommitsNode'; import { ResultsCommitsNode } from './resultsCommitsNode'; import type { FilesQueryResults } from './resultsFilesNode'; import { ResultsFilesNode } from './resultsFilesNode'; -import { ContextValues, getViewNodeId, ViewNode } from './viewNode'; +import type { ViewNode } from './viewNode'; +import { ContextValues, getViewNodeId, SubscribeableViewNode } from './viewNode'; let instanceId = 0; -export class CompareResultsNode extends ViewNode { - static getPinnableId(repoPath: string, ref1: string, ref2: string) { - return md5(`${repoPath}|${ref1}|${ref2}`, 'base64'); - } - +export class CompareResultsNode extends SubscribeableViewNode { private _instanceId: number; constructor( @@ -29,19 +28,36 @@ export class CompareResultsNode extends ViewNode { public readonly repoPath: string, private _ref: StoredNamedRef, private _compareWith: StoredNamedRef, - private _pinned: number = 0, + private _storedAt: number = 0, ) { super(GitUri.fromRepoPath(repoPath), view, parent); this._instanceId = instanceId++; - this.updateContext({ comparisonId: `${_ref.ref}+${_compareWith.ref}+${this._instanceId}` }); + this.updateContext({ + comparisonId: `${_ref.ref}+${_compareWith.ref}+${this._instanceId}`, + storedComparisonId: this.getStorageId(), + }); this._uniqueId = getViewNodeId('comparison-results', this.context); + + // If this is a new comparison, save it + if (this._storedAt === 0) { + this._storedAt = Date.now(); + void this.store(true); + } } override get id(): string { return this._uniqueId; } + protected override etag(): number { + return this._storedAt; + } + + get order(): number { + return this._storedAt; + } + get ahead(): { readonly ref1: string; readonly ref2: string } { return { ref1: this._compareWith.ref || 'HEAD', @@ -56,10 +72,6 @@ export class CompareResultsNode extends ViewNode { }; } - get canDismiss(): boolean { - return !this.pinned; - } - get compareRef(): StoredNamedRef { return this._ref; } @@ -68,13 +80,19 @@ export class CompareResultsNode extends ViewNode { return this._compareWith; } - private readonly _order: number = Date.now(); - get order(): number { - return this._pinned || this._order; + protected override subscribe(): Disposable | Promise | undefined { + return this.view.onDidChangeNodesCheckedState(this.onNodesCheckedStateChanged, this); } - get pinned(): boolean { - return this._pinned !== 0; + private onNodesCheckedStateChanged(e: TreeCheckboxChangeEvent) { + const prefix = getComparisonStoragePrefix(this.getStorageId()); + if (e.items.some(([n]) => n.id?.startsWith(prefix))) { + void this.store(true); + } + } + + dismiss() { + void this.remove(true); } private _children: ViewNode[] | undefined; @@ -165,13 +183,9 @@ export class CompareResultsNode extends ViewNode { TreeItemCollapsibleState.Collapsed, ); item.id = this.id; - item.contextValue = `${ContextValues.CompareResults}${this._pinned ? '+pinned' : ''}${ - this._ref.ref === '' ? '+working' : '' - }`; + item.contextValue = `${ContextValues.CompareResults}${this._ref.ref === '' ? '+working' : ''}`; item.description = description; - if (this._pinned) { - item.iconPath = new ThemeIcon('pinned'); - } + item.iconPath = new ThemeIcon('compare-changes'); return item; } @@ -182,16 +196,6 @@ export class CompareResultsNode extends ViewNode { return Promise.resolve<[string, string]>([this._compareWith.ref, this._ref.ref]); } - @log() - async pin() { - if (this.pinned) return; - - this._pinned = Date.now(); - await this.updatePinned(); - - queueMicrotask(() => this.view.reveal(this, { focus: true, select: true })); - } - @gate() @debug() override refresh(reset: boolean = false) { @@ -208,37 +212,20 @@ export class CompareResultsNode extends ViewNode { } // Save the current id so we can update it later - const currentId = this.getPinnableId(); + const currentId = this.getStorageId(); const ref1 = this._ref; this._ref = this._compareWith; this._compareWith = ref1; - // If we were pinned, remove the existing pin and save a new one - if (this.pinned) { - await this.view.updatePinned(currentId); - await this.updatePinned(); - } + // Remove the existing stored item and save a new one + await this.replace(currentId, true); this._children = undefined; this.view.triggerNodeChange(this.parent); queueMicrotask(() => this.view.reveal(this, { expand: true, focus: true, select: true })); } - @log() - async unpin() { - if (!this.pinned) return; - - this._pinned = 0; - await this.view.updatePinned(this.getPinnableId()); - - queueMicrotask(() => this.view.reveal(this, { focus: true, select: true })); - } - - private getPinnableId() { - return CompareResultsNode.getPinnableId(this.repoPath, this._ref.ref, this._compareWith.ref); - } - private async getAheadFilesQuery(): Promise { return this.getAheadBehindFilesQuery( createRevisionRange(this._compareWith?.ref || 'HEAD', this._ref.ref || 'HEAD', '...'), @@ -353,13 +340,62 @@ export class CompareResultsNode extends ViewNode { }; } - private updatePinned() { - return this.view.updatePinned(this.getPinnableId(), { - type: 'comparison', - timestamp: this._pinned, - path: this.repoPath, - ref1: { label: this._ref.label, ref: this._ref.ref }, - ref2: { label: this._compareWith.label, ref: this._compareWith.ref }, - }); + private getStorageId() { + return md5(`${this.repoPath}|${this._ref.ref}|${this._compareWith.ref}`, 'base64'); + } + + private remove(silent: boolean = false) { + return this.view.updateStorage(this.getStorageId(), undefined, silent); + } + + private async replace(id: string, silent: boolean = false) { + await this.view.updateStorage(id, undefined, silent); + return this.store(silent); + } + + store(silent = false) { + const storageId = this.getStorageId(); + const checkedFiles = getComparisonCheckedFiles(this.view, storageId); + + return this.view.updateStorage( + storageId, + { + type: 'comparison', + timestamp: this._storedAt, + path: this.repoPath, + ref1: { label: this._ref.label, ref: this._ref.ref }, + ref2: { label: this._compareWith.label, ref: this._compareWith.ref }, + checkedFiles: checkedFiles.length > 0 ? checkedFiles : undefined, + }, + silent, + ); + } +} + +export function getComparisonStoragePrefix(storageId: string) { + return `${storageId}|`; +} + +export function getComparisonCheckedFiles(view: View, storageId: string) { + const checkedFiles = []; + + const checked = view.nodeState.get(getComparisonStoragePrefix(storageId), 'checked'); + for (const [key, value] of checked) { + if (value === TreeItemCheckboxState.Checked) { + checkedFiles.push(key); + } + } + return checkedFiles; +} + +export function resetComparisonCheckedFiles(view: View, storageId: string) { + view.nodeState.delete(getComparisonStoragePrefix(storageId), 'checked'); +} + +export function restoreComparisonCheckedFiles(view: View, checkedFiles: string[] | undefined) { + if (checkedFiles?.length) { + for (const id of checkedFiles) { + view.nodeState.storeState(id, 'checked', TreeItemCheckboxState.Checked, true); + } } } diff --git a/src/views/nodes/resultsCommitsNode.ts b/src/views/nodes/resultsCommitsNode.ts index 768ca36..2a44aae 100644 --- a/src/views/nodes/resultsCommitsNode.ts +++ b/src/views/nodes/resultsCommitsNode.ts @@ -204,7 +204,7 @@ export class ResultsCommitsNode implements Fil super(GitUri.fromFile(file, repoPath, ref1 || ref2), view, parent, file); this.updateContext({ file: file }); - this._uniqueId = getViewNodeId('results-file', this.context); + if (this.context.storedComparisonId != null) { + this._uniqueId = `${getComparisonStoragePrefix(this.context.storedComparisonId)}${this.direction}|${ + file.path + }`; + } else { + this._uniqueId = getViewNodeId('results-file', this.context); + } } override toClipboard(): string { diff --git a/src/views/nodes/searchResultsNode.ts b/src/views/nodes/searchResultsNode.ts index 8846283..1a7c26f 100644 --- a/src/views/nodes/searchResultsNode.ts +++ b/src/views/nodes/searchResultsNode.ts @@ -4,10 +4,10 @@ import { md5 } from '@env/crypto'; import { executeGitCommand } from '../../git/actions'; import { GitUri } from '../../git/gitUri'; import type { GitLog } from '../../git/models/log'; -import type { SearchQuery, StoredSearchQuery } from '../../git/search'; +import type { SearchQuery } from '../../git/search'; import { getSearchQueryComparisonKey, getStoredSearchQuery } from '../../git/search'; import { gate } from '../../system/decorators/gate'; -import { debug, log } from '../../system/decorators/log'; +import { debug } from '../../system/decorators/log'; import { pluralize } from '../../system/string'; import type { SearchAndCompareView } from '../searchAndCompareView'; import type { CommitsQueryResults } from './resultsCommitsNode'; @@ -25,16 +25,13 @@ interface SearchQueryResults { } export class SearchResultsNode extends ViewNode implements PageableViewNode { - static getPinnableId(repoPath: string, search: SearchQuery | StoredSearchQuery) { - return md5(`${repoPath}|${getSearchQueryComparisonKey(search)}`, 'base64'); - } - private _instanceId: number; + constructor( view: SearchAndCompareView, protected override readonly parent: ViewNode, public readonly repoPath: string, - search: SearchQuery, + private _search: SearchQuery, private _labels: { label: string; queryLabel: @@ -50,40 +47,37 @@ export class SearchResultsNode extends ViewNode implements | Promise | GitLog | undefined, - private _pinned: number = 0, + private _storedAt: number = 0, ) { super(GitUri.fromRepoPath(repoPath), view, parent); - this._search = search; this._instanceId = instanceId++; - this._order = Date.now(); - - this.updateContext({ searchId: `${getSearchQueryComparisonKey(search)}+${this._instanceId}` }); + this.updateContext({ searchId: `${getSearchQueryComparisonKey(this._search)}+${this._instanceId}` }); this._uniqueId = getViewNodeId('search-results', this.context); + + // If this is a new search, save it + if (this._storedAt === 0) { + this._storedAt = Date.now(); + void this.store(true); + } } override get id(): string { return this._uniqueId; } - get canDismiss(): boolean { - return !this.pinned; - } - - private readonly _order: number = Date.now(); get order(): number { - return this._pinned || this._order; + return this._storedAt; } - get pinned(): boolean { - return this._pinned !== 0; - } - - private _search: SearchQuery; get search(): SearchQuery { return this._search; } + dismiss() { + void this.remove(true); + } + private _resultsNode: ResultsCommitsNode | undefined; private ensureResults() { if (this._resultsNode == null) { @@ -112,7 +106,7 @@ export class SearchResultsNode extends ViewNode implements deferred: deferred, }, { - expand: !this.pinned, + expand: false, }, true, ); @@ -128,14 +122,12 @@ export class SearchResultsNode extends ViewNode implements async getTreeItem(): Promise { const item = await this.ensureResults().getTreeItem(); item.id = this.id; - item.contextValue = `${ContextValues.SearchResults}${this._pinned ? '+pinned' : ''}`; + item.contextValue = ContextValues.SearchResults; if (this.view.container.git.repositoryCount > 1) { const repo = this.view.container.git.getRepository(this.repoPath); item.description = repo?.formattedName ?? this.repoPath; } - if (this._pinned) { - item.iconPath = new ThemeIcon('pinned'); - } + item.iconPath = new ThemeIcon('search'); return item; } @@ -177,18 +169,15 @@ export class SearchResultsNode extends ViewNode implements } // Save the current id so we can update it later - const currentId = this.getPinnableId(); + const currentId = this.getStorageId(); this._search = search.pattern; this._labels = search.labels; this._searchQueryOrLog = search.log; this._resultsNode = undefined; - // If we were pinned, remove the existing pin and save a new one - if (this.pinned) { - await this.view.updatePinned(currentId); - await this.updatePinned(); - } + // Remove the existing stored item and save a new one + await this.replace(currentId, true); void this.triggerChange(false); queueMicrotask(() => this.view.reveal(this, { expand: true, focus: true, select: true })); @@ -200,30 +189,6 @@ export class SearchResultsNode extends ViewNode implements this._resultsNode?.refresh(reset); } - @log() - async pin() { - if (this.pinned) return; - - this._pinned = Date.now(); - await this.updatePinned(); - - queueMicrotask(() => this.view.reveal(this, { focus: true, select: true })); - } - - @log() - async unpin() { - if (!this.pinned) return; - - this._pinned = 0; - await this.view.updatePinned(this.getPinnableId()); - - queueMicrotask(() => this.view.reveal(this, { focus: true, select: true })); - } - - private getPinnableId() { - return SearchResultsNode.getPinnableId(this.repoPath, this.search); - } - private getSearchLabel( label: | string @@ -238,7 +203,9 @@ export class SearchResultsNode extends ViewNode implements const count = log?.count ?? 0; const resultsType = - label.resultsType === undefined ? { singular: 'result', plural: 'results' } : label.resultsType; + label.resultsType === undefined + ? { singular: 'search result', plural: 'search results' } + : label.resultsType; return `${pluralize(resultsType.singular, count, { format: c => (log?.hasMore ? `${c}+` : undefined), @@ -286,13 +253,30 @@ export class SearchResultsNode extends ViewNode implements }; } - private updatePinned() { - return this.view.updatePinned(this.getPinnableId(), { - type: 'search', - timestamp: this._pinned, - path: this.repoPath, - labels: this._labels, - search: getStoredSearchQuery(this.search), - }); + private getStorageId() { + return md5(`${this.repoPath}|${getSearchQueryComparisonKey(this.search)}`, 'base64'); + } + + private remove(silent: boolean = false) { + return this.view.updateStorage(this.getStorageId(), undefined, silent); + } + + private async replace(id: string, silent: boolean = false) { + await this.view.updateStorage(id, undefined, silent); + return this.store(silent); + } + + private store(silent: boolean = false) { + return this.view.updateStorage( + this.getStorageId(), + { + type: 'search', + timestamp: this._storedAt, + path: this.repoPath, + labels: this._labels, + search: getStoredSearchQuery(this.search), + }, + silent, + ); } } diff --git a/src/views/nodes/viewNode.ts b/src/views/nodes/viewNode.ts index 6de3cf5..95935c2 100644 --- a/src/views/nodes/viewNode.ts +++ b/src/views/nodes/viewNode.ts @@ -28,7 +28,7 @@ import { debug, log, logName } from '../../system/decorators/log'; import { is as isA, szudzikPairing } from '../../system/function'; import { getLoggableName } from '../../system/logger'; import { pad } from '../../system/string'; -import type { TreeViewNodeCollapsibleStateChangeEvent, View } from '../viewBase'; +import type { View } from '../viewBase'; import type { BranchTrackingStatus } from './branchTrackingStatusNode'; export const enum ContextValues { @@ -111,6 +111,7 @@ export interface AmbientContext { readonly repository?: Repository; readonly root?: boolean; readonly searchId?: string; + readonly storedComparisonId?: string; readonly tag?: GitTag; readonly workspace?: CloudWorkspace | LocalWorkspace; readonly wsRepositoryDescriptor?: CloudWorkspaceRepositoryDescriptor | LocalWorkspaceRepositoryDescriptor; @@ -374,7 +375,7 @@ export abstract class SubscribeableViewNode extends V const disposables = [ this.view.onDidChangeVisibility(this.onVisibilityChanged, this), - this.view.onDidChangeNodeCollapsibleState(this.onNodeCollapsibleStateChanged, this), + // this.view.onDidChangeNodeCollapsibleState(this.onNodeCollapsibleStateChanged, this), ]; if (canAutoRefreshView(this.view)) { @@ -467,22 +468,22 @@ export abstract class SubscribeableViewNode extends V this.onVisibilityChanged({ visible: this.view.visible }); } - protected onParentCollapsibleStateChanged?(state: TreeItemCollapsibleState): void; - protected onCollapsibleStateChanged?(state: TreeItemCollapsibleState): void; - - protected collapsibleState: TreeItemCollapsibleState | undefined; - protected onNodeCollapsibleStateChanged(e: TreeViewNodeCollapsibleStateChangeEvent) { - if (e.element === this) { - this.collapsibleState = e.state; - if (this.onCollapsibleStateChanged !== undefined) { - this.onCollapsibleStateChanged(e.state); - } - } else if (e.element === this.parent) { - if (this.onParentCollapsibleStateChanged !== undefined) { - this.onParentCollapsibleStateChanged(e.state); - } - } - } + // protected onParentCollapsibleStateChanged?(state: TreeItemCollapsibleState): void; + // protected onCollapsibleStateChanged?(state: TreeItemCollapsibleState): void; + + // protected collapsibleState: TreeItemCollapsibleState | undefined; + // protected onNodeCollapsibleStateChanged(e: TreeViewNodeCollapsibleStateChangeEvent) { + // if (e.element === this) { + // this.collapsibleState = e.state; + // if (this.onCollapsibleStateChanged !== undefined) { + // this.onCollapsibleStateChanged(e.state); + // } + // } else if (e.element === this.parent) { + // if (this.onParentCollapsibleStateChanged !== undefined) { + // this.onParentCollapsibleStateChanged(e.state); + // } + // } + // } @debug() protected onVisibilityChanged(e: TreeViewVisibilityChangeEvent) { diff --git a/src/views/searchAndCompareView.ts b/src/views/searchAndCompareView.ts index d5c16e9..f7f4eae 100644 --- a/src/views/searchAndCompareView.ts +++ b/src/views/searchAndCompareView.ts @@ -2,7 +2,7 @@ import type { ConfigurationChangeEvent, Disposable } from 'vscode'; import { TreeItem, TreeItemCollapsibleState } from 'vscode'; import type { SearchAndCompareViewConfig } from '../config'; import { ViewFilesLayout } from '../config'; -import type { StoredNamedRef, StoredPinnedItem, StoredPinnedItems } from '../constants'; +import type { StoredNamedRef, StoredSearchAndCompareItem } from '../constants'; import { Commands } from '../constants'; import type { Container } from '../container'; import { unknownGitUri } from '../git/gitUri'; @@ -21,7 +21,7 @@ import { debug, log } from '../system/decorators/log'; import { updateRecordValue } from '../system/object'; import { isPromise } from '../system/promise'; import { ComparePickerNode } from './nodes/comparePickerNode'; -import { CompareResultsNode } from './nodes/compareResultsNode'; +import { CompareResultsNode, restoreComparisonCheckedFiles } from './nodes/compareResultsNode'; import { FilesQueryFilter, ResultsFilesNode } from './nodes/resultsFilesNode'; import { SearchResultsNode } from './nodes/searchResultsNode'; import { ContextValues, RepositoryFolderNode, ViewNode } from './nodes/viewNode'; @@ -41,10 +41,10 @@ export class SearchAndCompareViewNode extends ViewNode { if (this._children == null) { this._children = []; - // Get pinned searches & comparisons - const pinned = this.view.getPinned(); - if (pinned.length !== 0) { - this._children.push(...pinned); + // Get stored searches & comparisons + const stored = this.view.getStoredNodes(); + if (stored.length !== 0) { + this._children.push(...stored); } } @@ -56,7 +56,7 @@ export class SearchAndCompareViewNode extends ViewNode { this.view.message = undefined; - return this.children.sort((a, b) => (a.pinned ? -1 : 1) - (b.pinned ? -1 : 1) || b.order - a.order); + return this.children.sort((a, b) => b.order - a.order); } getTreeItem(): TreeItem { @@ -67,31 +67,24 @@ export class SearchAndCompareViewNode extends ViewNode { return item; } - addOrReplace(results: CompareResultsNode | SearchResultsNode, replace: boolean) { + addOrReplace(results: CompareResultsNode | SearchResultsNode) { if (this.children.includes(results)) return; - if (replace) { - this.clear(); - } - this.children.push(results); this.view.triggerNodeChange(); } @log() - clear(silent: boolean = false) { + async clear() { if (this.children.length === 0) return; this.removeComparePicker(true); - const index = this._children!.findIndex(c => !c.pinned); - if (index !== -1) { - this._children!.splice(index, this._children!.length); - } + this._children!.length = 0; - if (!silent) { - this.view.triggerNodeChange(); - } + await this.view.clearStorage(); + + this.view.triggerNodeChange(); } @log({ args: { 0: n => n.toString() } }) @@ -102,6 +95,10 @@ export class SearchAndCompareViewNode extends ViewNode { return; } + if (node instanceof CompareResultsNode || node instanceof SearchResultsNode) { + node.dismiss(); + } + if (this.children.length === 0) return; const index = this.children.indexOf(node); @@ -114,16 +111,16 @@ export class SearchAndCompareViewNode extends ViewNode { @gate() @debug() - override async refresh() { + override async refresh(reset: boolean = false) { if (this.children.length === 0) return; const promises: Promise[] = [ ...filterMap(this.children, c => { - const result = c.refresh === undefined ? false : c.refresh(); + const result = c.refresh?.(reset); return isPromise(result) ? result : undefined; }), ]; - await Promise.all(promises); + await Promise.allSettled(promises); } async compareWithSelected(repoPath?: string, ref?: string | StoredNamedRef) { @@ -254,8 +251,6 @@ export class SearchAndCompareView extends ViewBase< constructor(container: Container) { super(container, 'searchAndCompare', 'Search & Compare', 'searchAndCompareView'); - - void setContext('gitlens:views:searchAndCompare:keepResults', this.keepResults); } protected getRoot() { @@ -266,7 +261,7 @@ export class SearchAndCompareView extends ViewBase< void this.container.viewCommands; return [ - registerViewCommand(this.getQualifiedCommand('clear'), () => this.clear(), this), + registerViewCommand(this.getQualifiedCommand('clear'), () => void this.clear(), this), registerViewCommand( this.getQualifiedCommand('copy'), () => executeCommand(Commands.ViewsCopy, this.activeSelection, this.selection), @@ -288,17 +283,9 @@ export class SearchAndCompareView extends ViewBase< () => this.setFilesLayout(ViewFilesLayout.Tree), this, ), - registerViewCommand(this.getQualifiedCommand('setKeepResultsToOn'), () => this.setKeepResults(true), this), - registerViewCommand( - this.getQualifiedCommand('setKeepResultsToOff'), - () => this.setKeepResults(false), - this, - ), registerViewCommand(this.getQualifiedCommand('setShowAvatarsOn'), () => this.setShowAvatars(true), this), registerViewCommand(this.getQualifiedCommand('setShowAvatarsOff'), () => this.setShowAvatars(false), this), - registerViewCommand(this.getQualifiedCommand('pin'), this.pin, this), - registerViewCommand(this.getQualifiedCommand('unpin'), this.unpin, this), registerViewCommand(this.getQualifiedCommand('swapComparison'), this.swapComparison, this), registerViewCommand(this.getQualifiedCommand('selectForCompare'), () => this.selectForCompare()), registerViewCommand(this.getQualifiedCommand('compareWithSelected'), this.compareWithSelected, this), @@ -339,12 +326,8 @@ export class SearchAndCompareView extends ViewBase< return true; } - get keepResults(): boolean { - return this.container.storage.getWorkspace('views:searchAndCompare:keepResults', true); - } - clear() { - this.root?.clear(); + return this.root?.clear(); } dismissNode(node: ViewNode) { @@ -352,8 +335,7 @@ export class SearchAndCompareView extends ViewBase< this.root == null || (!(node instanceof ComparePickerNode) && !(node instanceof CompareResultsNode) && - !(node instanceof SearchResultsNode)) || - !node.canDismiss + !(node instanceof SearchResultsNode)) ) { return; } @@ -407,7 +389,10 @@ export class SearchAndCompareView extends ViewBase< await this.show(); } - const labels = { label: `Results ${typeof label === 'string' ? label : label.label}`, queryLabel: label }; + const labels = { + label: `Search results ${typeof label === 'string' ? label : label.label}`, + queryLabel: label, + }; if (updateNode != null) { await updateNode.edit({ pattern: search, labels: labels, log: results }); @@ -417,44 +402,16 @@ export class SearchAndCompareView extends ViewBase< await this.addResults(new SearchResultsNode(this, this.root!, repoPath, search, labels, results), reveal); } - getPinned() { - let savedPins = this.container.storage.getWorkspace('views:searchAndCompare:pinned'); - if (savedPins == null) { - // Migrate any deprecated pinned items - const deprecatedPins = this.container.storage.getWorkspace('pinned:comparisons'); - if (deprecatedPins == null) return []; - - savedPins = Object.create(null) as StoredPinnedItems; - for (const p of Object.values(deprecatedPins)) { - savedPins[CompareResultsNode.getPinnableId(p.path, p.ref1.ref, p.ref2.ref)] = { - type: 'comparison', - timestamp: Date.now(), - path: p.path, - ref1: p.ref1, - ref2: p.ref2, - }; - } - - void this.container.storage.storeWorkspace('views:searchAndCompare:pinned', savedPins); - void this.container.storage.deleteWorkspace('pinned:comparisons'); - } - - const migratedPins = Object.create(null) as StoredPinnedItems; - let migrated = false; + getStoredNodes() { + const stored = this.container.storage.getWorkspace('views:searchAndCompare:pinned'); + if (stored == null) return []; const root = this.ensureRoot(); - const pins = Object.entries(savedPins) + const nodes = Object.entries(stored) .sort(([, a], [, b]) => (b.timestamp ?? 0) - (a.timestamp ?? 0)) - .map(([k, p]) => { + .map(([, p]) => { if (p.type === 'comparison') { - // Migrated any old keys (sha1) to new keys (md5) - const key = CompareResultsNode.getPinnableId(p.path, p.ref1.ref, p.ref2.ref); - if (k !== key) { - migrated = true; - migratedPins[key] = p; - } else { - migratedPins[k] = p; - } + restoreComparisonCheckedFiles(this, p.checkedFiles); return new CompareResultsNode( this, @@ -466,15 +423,6 @@ export class SearchAndCompareView extends ViewBase< ); } - // Migrated any old keys (sha1) to new keys (md5) - const key = SearchResultsNode.getPinnableId(p.path, p.search); - if (k !== key) { - migrated = true; - migratedPins[key] = p; - } else { - migratedPins[k] = p; - } - return new SearchResultsNode( this, root, @@ -486,18 +434,21 @@ export class SearchAndCompareView extends ViewBase< ); }); - if (migrated) { - void this.container.storage.storeWorkspace('views:searchAndCompare:pinned', migratedPins); - } - return pins; + return nodes; } - async updatePinned(id: string, pin?: StoredPinnedItem) { - let pinned = this.container.storage.getWorkspace('views:searchAndCompare:pinned'); - pinned = updateRecordValue(pinned, id, pin); - await this.container.storage.storeWorkspace('views:searchAndCompare:pinned', pinned); + clearStorage() { + return this.container.storage.deleteWorkspace('views:searchAndCompare:pinned'); + } + + async updateStorage(id: string, item?: StoredSearchAndCompareItem, silent: boolean = false) { + let stored = this.container.storage.getWorkspace('views:searchAndCompare:pinned'); + stored = updateRecordValue(stored, id, item); + await this.container.storage.storeWorkspace('views:searchAndCompare:pinned', stored); - this.triggerNodeChange(this.ensureRoot()); + if (!silent) { + this.triggerNodeChange(this.ensureRoot()); + } } @gate(() => '') @@ -530,7 +481,7 @@ export class SearchAndCompareView extends ViewBase< } const root = this.ensureRoot(); - root.addOrReplace(results, !this.keepResults); + root.addOrReplace(results); queueMicrotask(() => this.reveal(results, options)); } @@ -539,21 +490,10 @@ export class SearchAndCompareView extends ViewBase< return configuration.updateEffective(`views.${this.configKey}.files.layout` as const, layout); } - private setKeepResults(enabled: boolean) { - void this.container.storage.storeWorkspace('views:searchAndCompare:keepResults', enabled); - void setContext('gitlens:views:searchAndCompare:keepResults', enabled); - } - private setShowAvatars(enabled: boolean) { return configuration.updateEffective(`views.${this.configKey}.avatars` as const, enabled); } - private pin(node: CompareResultsNode | SearchResultsNode) { - if (!(node instanceof CompareResultsNode) && !(node instanceof SearchResultsNode)) return undefined; - - return node.pin(); - } - private setFilesFilter(node: ResultsFilesNode, filter: FilesQueryFilter | undefined) { if (!(node instanceof ResultsFilesNode)) return; @@ -565,10 +505,4 @@ export class SearchAndCompareView extends ViewBase< return node.swap(); } - - private unpin(node: CompareResultsNode | SearchResultsNode) { - if (!(node instanceof CompareResultsNode) && !(node instanceof SearchResultsNode)) return undefined; - - return node.unpin(); - } } diff --git a/src/views/viewBase.ts b/src/views/viewBase.ts index 80261bf..696c94d 100644 --- a/src/views/viewBase.ts +++ b/src/views/viewBase.ts @@ -135,6 +135,11 @@ export abstract class ViewBase< return this._onDidChangeNodeCollapsibleState.event; } + private _onDidChangeNodesCheckedState = new EventEmitter>(); + get onDidChangeNodesCheckedState(): Event> { + return this._onDidChangeNodesCheckedState.event; + } + protected disposables: Disposable[] = []; protected root: RootNode | undefined; protected tree: TreeView | undefined; @@ -367,13 +372,17 @@ export abstract class ViewBase< } protected onCheckboxStateChanged(e: TreeCheckboxChangeEvent) { - for (const [node, state] of e.items) { - if (node.id == null) { - debugger; - throw new Error('Id is required for checkboxes'); - } + try { + for (const [node, state] of e.items) { + if (node.id == null) { + debugger; + throw new Error('Id is required for checkboxes'); + } - node.storeState('checked', state, true); + node.storeState('checked', state, true); + } + } finally { + this._onDidChangeNodesCheckedState.fire(e); } } @@ -681,6 +690,18 @@ export class ViewNodeState implements Disposable { this._store = undefined; } + delete(prefix: string, key: string): void { + for (const store of [this._store, this._stickyStore]) { + if (store == null) continue; + + for (const [id, map] of store) { + if (id.startsWith(prefix)) { + map.delete(key); + } + } + } + } + deleteState(id: string, key?: string): void { if (key == null) { this._store?.delete(id); @@ -691,6 +712,22 @@ export class ViewNodeState implements Disposable { } } + get(prefix: string, key: string): Map { + const maps = new Map(); + + for (const store of [this._store, this._stickyStore]) { + if (store == null) continue; + + for (const [id, map] of store) { + if (id.startsWith(prefix) && map.has(key)) { + maps.set(id, map.get(key) as T); + } + } + } + + return maps; + } + getState(id: string, key: string): T | undefined { return (this._stickyStore?.get(id)?.get(key) ?? this._store?.get(id)?.get(key)) as T | undefined; }