From 2a7c0c40bbc9f18e0c1be06501b0032632192560 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 20 Jan 2021 01:59:12 -0500 Subject: [PATCH] Adds more granular repo file watching Adds repo change events for rebase, merge, and cherry-pick Clears some caches only when specific change occurs --- src/git/gitService.ts | 39 ++++++---- src/git/models/repository.ts | 152 ++++++++++++++++++++++++------------- src/trackers/trackedDocument.ts | 18 ++++- src/views/branchesView.ts | 15 ++-- src/views/commitsView.ts | 15 ++-- src/views/contributorsView.ts | 13 ++-- src/views/nodes/fileHistoryNode.ts | 15 ++-- src/views/nodes/lineHistoryNode.ts | 15 ++-- src/views/nodes/repositoryNode.ts | 32 ++++---- src/views/nodes/viewNode.ts | 8 +- src/views/remotesView.ts | 10 ++- src/views/stashesView.ts | 14 ++-- src/views/tagsView.ts | 14 ++-- src/webviews/rebaseEditor.ts | 13 +--- 14 files changed, 229 insertions(+), 144 deletions(-) diff --git a/src/git/gitService.ts b/src/git/gitService.ts index cc9de2e..5361ad4 100644 --- a/src/git/gitService.ts +++ b/src/git/gitService.ts @@ -72,6 +72,7 @@ import { PullRequestState, Repository, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, SearchPattern, } from './git'; @@ -188,32 +189,40 @@ export class GitService implements Disposable { } private onAnyRepositoryChanged(repo: Repository, e: RepositoryChangeEvent) { - if (e.changed(RepositoryChange.Stash, true)) { - this._stashesCache.delete(repo.path); + if (e.changed(RepositoryChange.Config, RepositoryChangeComparisonMode.Any)) { + this._userMapCache.delete(repo.path); + } - return; + if (e.changed(RepositoryChange.Heads, RepositoryChange.Remotes, RepositoryChangeComparisonMode.Any)) { + this._branchesCache.delete(repo.path); + this._contributorsCache.delete(repo.path); + + if (e.changed(RepositoryChange.Remotes, RepositoryChangeComparisonMode.Any)) { + this._remotesWithApiProviderCache.clear(); + } } - this._branchesCache.delete(repo.path); - this._contributorsCache.delete(repo.path); - this._mergeStatusCache.delete(repo.path); - this._rebaseStatusCache.delete(repo.path); - this._tagsCache.delete(repo.path); - this._trackedCache.clear(); + if (e.changed(RepositoryChange.Index, RepositoryChange.Unknown, RepositoryChangeComparisonMode.Any)) { + this._trackedCache.clear(); + } - if (e.changed(RepositoryChange.Remotes)) { - this._remotesWithApiProviderCache.clear(); + if (e.changed(RepositoryChange.Merge, RepositoryChangeComparisonMode.Any)) { + this._mergeStatusCache.delete(repo.path); } - if (e.changed(RepositoryChange.Stash)) { + if (e.changed(RepositoryChange.Rebase, RepositoryChangeComparisonMode.Any)) { + this._rebaseStatusCache.delete(repo.path); + } + + if (e.changed(RepositoryChange.Stash, RepositoryChangeComparisonMode.Any)) { this._stashesCache.delete(repo.path); } - if (e.changed(RepositoryChange.Config)) { - this._userMapCache.delete(repo.path); + if (e.changed(RepositoryChange.Tags, RepositoryChangeComparisonMode.Any)) { + this._tagsCache.delete(repo.path); } - if (e.changed(RepositoryChange.Closed)) { + if (e.changed(RepositoryChange.Closed, RepositoryChangeComparisonMode.Any)) { // Send a notification that the repositories changed setImmediate(async () => { await this.updateContext(this._repositoryTree); diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index 75f6561..f513bbd 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -43,58 +43,98 @@ import { RemoteProviderFactory, RemoteProviders, RichRemoteProvider } from '../r import { Arrays, Dates, debug, Functions, gate, Iterables, log, logName } from '../../system'; import { runGitCommandInTerminal } from '../../terminal'; -const ignoreGitRegex = /\.git(?:\/|\\|$)/; -const refsRegex = /\.git\/refs\/(heads|remotes|tags)/; +export const enum RepositoryChange { + // FileSystem = 'filesystem', + Unknown = 'unknown', -export enum RepositoryChange { - Config = 'config', + // No file watching required Closed = 'closed', - // FileSystem = 'file-system', + Ignores = 'ignores', Starred = 'starred', + + // File watching required + CherryPick = 'cherrypick', + Config = 'config', Heads = 'heads', Index = 'index', - Ignores = 'ignores', + Merge = 'merge', + Rebase = 'rebase', Remotes = 'remotes', Stash = 'stash', + /* + * Union of Cherry, Merge, and Rebase + */ + Status = 'status', Tags = 'tags', - Unknown = 'unknown', +} + +export const enum RepositoryChangeComparisonMode { + Any, + All, + Exclusive, } export class RepositoryChangeEvent { - constructor(public readonly repository?: Repository, public readonly changes: RepositoryChange[] = []) {} - - changed(change: RepositoryChange, only: boolean = false) { - if (only) { - if (this.changes.length !== 1) return false; - if (this.changes[0] === change) return true; - - if (this.repository?.supportsChangeEvents === false && this.changes[0] === RepositoryChange.Unknown) { - switch (change) { - case RepositoryChange.Closed: - case RepositoryChange.Starred: - return false; - default: - return true; - } + private readonly _changes: Set; + + constructor(public readonly repository: Repository, changes: RepositoryChange[]) { + this._changes = new Set(changes); + } + + toString(changesOnly: boolean = false): string { + return changesOnly + ? `changes=${Iterables.join(this._changes, ', ')}` + : `{ repository: ${this.repository?.name ?? ''}, changes: ${Iterables.join(this._changes, ', ')} }`; + } + + changed(...args: [...RepositoryChange[], RepositoryChangeComparisonMode]) { + let affected = args.slice(0, -1) as RepositoryChange[]; + const mode = args[args.length - 1] as RepositoryChangeComparisonMode; + + // If we don't support file watching, then treat Unknown as acceptable for any change other than Closed/Ignores/Starred, i.e. any changes that require file watching + if (!this.repository.supportsChangeEvents) { + if (this._changes.has(RepositoryChange.Unknown)) { + affected = affected.filter( + c => + c === RepositoryChange.Closed || + c === RepositoryChange.Ignores || + c === RepositoryChange.Starred, + ); + if (affected.length === 0) return true; } + } - return false; + if (mode === RepositoryChangeComparisonMode.Any) { + return Iterables.some(this._changes, c => affected.includes(c)); } - if (this.changes.includes(change)) return true; + let changes = this._changes; - if (this.repository?.supportsChangeEvents === false && this.changes.includes(RepositoryChange.Unknown)) { - switch (change) { - case RepositoryChange.Closed: - case RepositoryChange.Ignores: - case RepositoryChange.Starred: - return false; - default: - return true; + if (mode === RepositoryChangeComparisonMode.Exclusive) { + if ( + affected.includes(RepositoryChange.CherryPick) || + affected.includes(RepositoryChange.Merge) || + affected.includes(RepositoryChange.Rebase) + ) { + if (!affected.includes(RepositoryChange.Status)) { + affected.push(RepositoryChange.Status); + } + } else if (affected.includes(RepositoryChange.Status)) { + changes = new Set(changes); + changes.delete(RepositoryChange.CherryPick); + changes.delete(RepositoryChange.Merge); + changes.delete(RepositoryChange.Rebase); } } - return false; + const intersection = [...Iterables.filter(changes, c => affected.includes(c))]; + return mode === RepositoryChangeComparisonMode.Exclusive + ? intersection.length === changes.size + : intersection.length === affected.length; + } + + with(changes: RepositoryChange[]) { + return new RepositoryChangeEvent(this.repository, [...this._changes, ...changes]); } } @@ -205,10 +245,8 @@ export class Repository implements Disposable { **/.git/config,\ **/.git/index,\ **/.git/HEAD,\ -**/.git/refs/stash,\ -**/.git/refs/heads/**,\ -**/.git/refs/remotes/**,\ -**/.git/refs/tags/**,\ +**/.git/*_HEAD,\ +**/.git/refs/**,\ **/.gitignore\ }', ), @@ -275,7 +313,7 @@ export class Repository implements Disposable { private onFileSystemChanged(uri: Uri) { // Ignore .git changes - if (ignoreGitRegex.test(uri.fsPath)) return; + if (/\.git(?:\/|\\|$)/.test(uri.fsPath)) return; this.fireFileSystemChange(uri); } @@ -316,13 +354,31 @@ export class Repository implements Disposable { return; } + if (uri.path.endsWith('.git/CHERRY_PICK_HEAD')) { + this.fireChange(RepositoryChange.CherryPick, RepositoryChange.Status); + + return; + } + + if (uri.path.endsWith('.git/MERGE_HEAD')) { + this.fireChange(RepositoryChange.Merge, RepositoryChange.Status); + + return; + } + + if (uri.path.endsWith('.git/REBASE_HEAD') || /\.git\/rebase-merge/.test(uri.path)) { + this.fireChange(RepositoryChange.Rebase, RepositoryChange.Status); + + return; + } + if (uri.path.endsWith('/.gitignore')) { this.fireChange(RepositoryChange.Ignores); return; } - const match = refsRegex.exec(uri.path); + const match = /\.git\/refs\/(heads|remotes|tags)/.exec(uri.path); if (match != null) { switch (match[1]) { case 'heads': @@ -919,22 +975,14 @@ export class Repository implements Disposable { this._fireChangeDebounced = Functions.debounce(this.fireChangeCore.bind(this), 250); } - if (this._pendingRepoChange == null) { - this._pendingRepoChange = new RepositoryChangeEvent(this); - } - - const e = this._pendingRepoChange; - - for (const reason of changes) { - if (!e.changes.includes(reason)) { - e.changes.push(reason); - } - } + this._pendingRepoChange = this._pendingRepoChange?.with(changes) ?? new RepositoryChangeEvent(this, changes); this.onAnyRepositoryChanged(this, new RepositoryChangeEvent(this, changes)); if (this._suspended) { - Logger.debug(`Repository[${this.name}(${this.id})] queueing suspended changes=${e.changes.join(', ')}`); + Logger.debug( + `Repository[${this.name}(${this.id})] queueing suspended ${this._pendingRepoChange.toString(true)}`, + ); return; } @@ -948,7 +996,7 @@ export class Repository implements Disposable { this._pendingRepoChange = undefined; - Logger.debug(`Repository[${this.name}(${this.id})] firing changes=${e.changes.join(', ')}`); + Logger.debug(`Repository[${this.name}(${this.id})] firing ${e.toString(true)}`); this._onDidChange.fire(e); } diff --git a/src/trackers/trackedDocument.ts b/src/trackers/trackedDocument.ts index a0653af..1637afd 100644 --- a/src/trackers/trackedDocument.ts +++ b/src/trackers/trackedDocument.ts @@ -2,7 +2,13 @@ import { Disposable, Event, EventEmitter, TextDocument, TextEditor } from 'vscode'; import { ContextKeys, getEditorIfActive, isActiveDocument, setContext } from '../constants'; import { Container } from '../container'; -import { GitRevision, Repository, RepositoryChange, RepositoryChangeEvent } from '../git/git'; +import { + GitRevision, + Repository, + RepositoryChange, + RepositoryChangeComparisonMode, + RepositoryChangeEvent, +} from '../git/git'; import { GitUri } from '../git/gitUri'; import { Logger } from '../logger'; import { Functions } from '../system'; @@ -85,9 +91,13 @@ export class TrackedDocument implements Disposable { private onRepositoryChanged(e: RepositoryChangeEvent) { if ( - !e.changed(RepositoryChange.Index) && - !e.changed(RepositoryChange.Heads) && - !e.changed(RepositoryChange.Unknown) + !e.changed( + RepositoryChange.Index, + RepositoryChange.Heads, + RepositoryChange.Status, + RepositoryChange.Unknown, + RepositoryChangeComparisonMode.Any, + ) ) { return; } diff --git a/src/views/branchesView.ts b/src/views/branchesView.ts index 8119f95..cf7f138 100644 --- a/src/views/branchesView.ts +++ b/src/views/branchesView.ts @@ -23,6 +23,7 @@ import { GitReference, GitRevisionReference, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, } from '../git/git'; import { GitUri } from '../git/gitUri'; @@ -48,12 +49,14 @@ export class BranchesRepositoryNode extends RepositoryFolderNode impl private onRepositoryChanged(e: RepositoryChangeEvent) { if ( - !e.changed(RepositoryChange.Index) && - !e.changed(RepositoryChange.Heads) && - !e.changed(RepositoryChange.Remotes) && - !e.changed(RepositoryChange.Unknown) + !e.changed( + RepositoryChange.Index, + RepositoryChange.Heads, + RepositoryChange.Remotes, + RepositoryChange.Status, + RepositoryChange.Unknown, + RepositoryChangeComparisonMode.Any, + ) ) { return; } - Logger.debug(`FileHistoryNode.onRepositoryChanged(${e.changes.join()}); triggering node refresh`); + Logger.debug(`FileHistoryNode.onRepositoryChanged(${e.toString()}); triggering node refresh`); void this.triggerChange(true); } diff --git a/src/views/nodes/lineHistoryNode.ts b/src/views/nodes/lineHistoryNode.ts index f8f3217..b4f06ff 100644 --- a/src/views/nodes/lineHistoryNode.ts +++ b/src/views/nodes/lineHistoryNode.ts @@ -13,6 +13,7 @@ import { GitLogCommit, GitRevision, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, RepositoryFileSystemChangeEvent, } from '../../git/git'; @@ -275,15 +276,19 @@ export class LineHistoryNode private onRepositoryChanged(e: RepositoryChangeEvent) { if ( - !e.changed(RepositoryChange.Index) && - !e.changed(RepositoryChange.Heads) && - !e.changed(RepositoryChange.Remotes) && - !e.changed(RepositoryChange.Unknown) + !e.changed( + RepositoryChange.Index, + RepositoryChange.Heads, + RepositoryChange.Remotes, + RepositoryChange.Status, + RepositoryChange.Unknown, + RepositoryChangeComparisonMode.Any, + ) ) { return; } - Logger.debug(`LineHistoryNode.onRepositoryChanged(${e.changes.join()}); triggering node refresh`); + Logger.debug(`LineHistoryNode.onRepositoryChanged(${e.toString()}); triggering node refresh`); void this.triggerChange(true); } diff --git a/src/views/nodes/repositoryNode.ts b/src/views/nodes/repositoryNode.ts index fd6346d..9a0d1d8 100644 --- a/src/views/nodes/repositoryNode.ts +++ b/src/views/nodes/repositoryNode.ts @@ -8,6 +8,7 @@ import { GitStatus, Repository, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, RepositoryFileSystemChangeEvent, } from '../../git/git'; @@ -392,48 +393,51 @@ export class RepositoryNode extends SubscribeableViewNode { @debug({ args: { - 0: (e: RepositoryChangeEvent) => - `{ repository: ${e.repository?.name ?? ''}, changes: ${e.changes.join()} }`, + 0: (e: RepositoryChangeEvent) => e.toString(), }, }) private onRepositoryChanged(e: RepositoryChangeEvent) { this._repoUpdatedAt = this.repo.updatedAt; - if (e.changed(RepositoryChange.Closed)) { + if (e.changed(RepositoryChange.Closed, RepositoryChangeComparisonMode.Any)) { this.dispose(); return; } if ( - this._children === undefined || - e.changed(RepositoryChange.Config) || - e.changed(RepositoryChange.Index) || - e.changed(RepositoryChange.Heads) || - e.changed(RepositoryChange.Unknown) + this._children == null || + e.changed( + RepositoryChange.Config, + RepositoryChange.Index, + RepositoryChange.Heads, + RepositoryChange.Status, + RepositoryChange.Unknown, + RepositoryChangeComparisonMode.Any, + ) ) { void this.triggerChange(true); return; } - if (e.changed(RepositoryChange.Remotes)) { + if (e.changed(RepositoryChange.Remotes, RepositoryChangeComparisonMode.Any)) { const node = this._children.find(c => c instanceof RemotesNode); - if (node !== undefined) { + if (node != null) { void this.view.triggerNodeChange(node); } } - if (e.changed(RepositoryChange.Stash)) { + if (e.changed(RepositoryChange.Stash, RepositoryChangeComparisonMode.Any)) { const node = this._children.find(c => c instanceof StashesNode); - if (node !== undefined) { + if (node != null) { void this.view.triggerNodeChange(node); } } - if (e.changed(RepositoryChange.Tags)) { + if (e.changed(RepositoryChange.Tags, RepositoryChangeComparisonMode.Any)) { const node = this._children.find(c => c instanceof TagsNode); - if (node !== undefined) { + if (node != null) { void this.view.triggerNodeChange(node); } } diff --git a/src/views/nodes/viewNode.ts b/src/views/nodes/viewNode.ts index 52c6a2a..1ec79df 100644 --- a/src/views/nodes/viewNode.ts +++ b/src/views/nodes/viewNode.ts @@ -8,6 +8,7 @@ import { GitRevisionReference, Repository, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, } from '../../git/git'; import { GitUri } from '../../git/gitUri'; @@ -408,21 +409,20 @@ export abstract class RepositoryFolderNode< @debug({ args: { - 0: (e: RepositoryChangeEvent) => - `{ repository: ${e.repository?.name ?? ''}, changes: ${e.changes.join()} }`, + 0: (e: RepositoryChangeEvent) => e.toString(), }, }) private onRepositoryChanged(e: RepositoryChangeEvent) { this._repoUpdatedAt = this.repo.updatedAt; - if (e.changed(RepositoryChange.Closed)) { + if (e.changed(RepositoryChange.Closed, RepositoryChangeComparisonMode.Any)) { this.dispose(); void this.parent?.triggerChange(true); return; } - if (e.changed(RepositoryChange.Starred)) { + if (e.changed(RepositoryChange.Starred, RepositoryChangeComparisonMode.Any)) { void this.parent?.triggerChange(true); return; diff --git a/src/views/remotesView.ts b/src/views/remotesView.ts index d86a315..574a255 100644 --- a/src/views/remotesView.ts +++ b/src/views/remotesView.ts @@ -19,6 +19,7 @@ import { GitRemote, GitRevisionReference, RepositoryChange, + RepositoryChangeComparisonMode, RepositoryChangeEvent, } from '../git/git'; import { GitUri } from '../git/gitUri'; @@ -45,10 +46,11 @@ export class RemotesRepositoryNode extends RepositoryFolderNode } protected changed(e: RepositoryChangeEvent) { - return ( - e.changed(RepositoryChange.Config) || - e.changed(RepositoryChange.Tags) || - e.changed(RepositoryChange.Unknown) - ); + return e.changed(RepositoryChange.Tags, RepositoryChange.Unknown, RepositoryChangeComparisonMode.Any); } } diff --git a/src/webviews/rebaseEditor.ts b/src/webviews/rebaseEditor.ts index ff529f7..7e66fb6 100644 --- a/src/webviews/rebaseEditor.ts +++ b/src/webviews/rebaseEditor.ts @@ -19,7 +19,7 @@ import { ShowQuickCommitCommand } from '../commands'; import { configuration } from '../configuration'; import { BuiltInCommands } from '../constants'; import { Container } from '../container'; -import { Repository, RepositoryChange } from '../git/git'; +import { Repository, RepositoryChange, RepositoryChangeComparisonMode } from '../git/git'; import { Logger } from '../logger'; import { Messages } from '../messages'; import { debug, gate, Iterables } from '../system'; @@ -211,16 +211,7 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl void this.getStateAndNotify(context); }), repo.onDidChange(e => { - if ( - e.changed(RepositoryChange.Closed, true) || - e.changed(RepositoryChange.Ignores, true) || - e.changed(RepositoryChange.Remotes, true) || - e.changed(RepositoryChange.Starred, true) || - e.changed(RepositoryChange.Stash, true) || - e.changed(RepositoryChange.Tags, true) - ) { - return; - } + if (!e.changed(RepositoryChange.Rebase, RepositoryChangeComparisonMode.Any)) return; void this.getStateAndNotify(context); }),