(WIP) Refactoring out view nodes to better deal with id collisions and storage

src/views/branchesView.ts

@@ -76,7 +76,7 @@ export class BranchesViewNode extends RepositoriesSubscribeableNode<BranchesView
 
 			const splat = repositories.length === 1;
 			this.children = repositories.map(
-				r => new BranchesRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, r, splat),
+				r => new BranchesRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, splat, r),
 			);
 		}
 

src/views/commitsView.ts

@@ -136,7 +136,7 @@ export class CommitsViewNode extends RepositoriesSubscribeableNode<CommitsView,
 			const splat = repositories.length === 1;
 			this.children = repositories.map(
 				r =>
-					new CommitsRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, r, splat, {
+					new CommitsRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, splat, r, {
 						showBranchAndLastFetched: true,
 					}),
 			);

src/views/contributorsView.ts

@@ -76,7 +76,7 @@ export class ContributorsViewNode extends RepositoriesSubscribeableNode<Contribu
 
 			const splat = repositories.length === 1;
 			this.children = repositories.map(
-				r => new ContributorsRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, r, splat),
+				r => new ContributorsRepositoryNode(GitUri.fromRepoPath(r.path), this.view, this, splat, r),
 			);
 		}
 

src/views/launchpadView.ts

@@ -46,10 +46,6 @@ export class LaunchpadItemNode extends CacheableChildrenViewNode<'launchpad-item
 		this.repoPath = repoPath;
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	override toClipboard(type?: ClipboardType): string {
 		const url = this.getUrl();
 		switch (type) {

src/views/nodes/UncommittedFilesNode.ts

@@ -31,10 +31,6 @@ export class UncommittedFilesNode extends ViewNode<'uncommitted-files', ViewsWit
 		this._uniqueId = getViewNodeId(this.type, this.context);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	get repoPath(): string {
 		return this.status.repoPath;
 	}

src/views/nodes/abstract/repositoriesSubscribeableNode.ts

@@ -13,10 +13,8 @@ export abstract class RepositoriesSubscribeableNode<
 	TView extends View = View,
 	TChild extends ViewNode = ViewNode,
 > extends SubscribeableViewNode<'repositories', TView, TChild> {
-	protected override splatted = true;
-
 	constructor(view: TView) {
-		super('repositories', unknownGitUri, view);
+		super('repositories', unknownGitUri, view, undefined, true);
 	}
 
 	override async getSplattedChild() {

src/views/nodes/abstract/repositoryFolderNode.ts

@@ -19,22 +19,18 @@ export abstract class RepositoryFolderNode<
 	TView extends View = View,
 	TChild extends ViewNode = ViewNode,
 > extends SubscribeableViewNode<'repo-folder', TView> {
-	protected override splatted = true;
-
 	constructor(
 		uri: GitUri,
 		view: TView,
 		protected override readonly parent: ViewNode,
-		public readonly repo: Repository,
 		splatted: boolean,
+		public readonly repo: Repository,
 		private readonly options?: { showBranchAndLastFetched?: boolean },
 	) {
-		super('repo-folder', uri, view, parent);
+		super('repo-folder', uri, view, parent, splatted);
 
 		this.updateContext({ repository: this.repo });
 		this._uniqueId = getViewNodeId(this.type, this.context);
-
-		this.splatted = splatted;
 	}
 
 	private _child: TChild | undefined;
@@ -53,10 +49,6 @@ export abstract class RepositoryFolderNode<
 		this.child = undefined;
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	override toClipboard(): string {
 		return this.repo.path;
 	}
@@ -66,8 +58,6 @@ export abstract class RepositoryFolderNode<
 	}
 
 	async getTreeItem(): Promise<TreeItem> {
-		this.splatted = false;
-
 		const branch = await this.repo.git.getBranch();
 		const ahead = (branch?.state.ahead ?? 0) > 0;
 		const behind = (branch?.state.behind ?? 0) > 0;

src/views/nodes/abstract/subscribeableViewNode.ts

@@ -21,8 +21,8 @@ export abstract class SubscribeableViewNode<
 
 	protected loaded: boolean = false;
 
-	constructor(type: Type, uri: GitUri, view: TView, parent?: ViewNode) {
-		super(type, uri, view, parent);
+	constructor(type: Type, uri: GitUri, view: TView, parent?: ViewNode, splatted?: boolean) {
+		super(type, uri, view, parent, splatted);
 
 		const disposables = [
 			weakEvent(this.view.onDidChangeVisibility, this.onVisibilityChanged, this),

src/views/nodes/abstract/viewNode.ts

@@ -154,7 +154,29 @@ export interface AmbientContext {
 	readonly worktreesByBranch?: Map<string, GitWorktree>;
 }
 
-export function getViewNodeId(type: string, context: AmbientContext): string {
+export function getViewNodeId(
+	type: TreeViewNodeTypes | `${TreeViewNodeTypes}+${string}`,
+	context: AmbientContext,
+): string {
+	switch (type) {
+		case 'branch':
+			return `${type}(${context.branch?.id})`;
+
+		case 'commit':
+			return `${type}(${context.commit?.repoPath}|${context.commit?.sha})`;
+
+		case 'pullrequest':
+			return `${type}(${context.pullRequest?.url})`;
+
+		case 'commit-file':
+			return `${type}:(${
+				context.repository?.path ?? context.branch?.repoPath ?? context.commit?.repoPath
+			}|${context.file?.path}+${context.file?.status})`;
+
+		// case 'results-file':
+		// 	return `${type}(${context.file?.path}+${context.file?.status})`;
+	}
+
 	let uniqueness = '';
 	if (context.root) {
 		uniqueness += '/root';
@@ -252,8 +274,22 @@ export abstract class ViewNode<
 		return types.includes(this.type as unknown as T[number]);
 	}
 
+	public childrenIds = new Set<string>();
+	public childrenCount = 0;
 	protected _uniqueId!: string;
-	protected splatted = false;
+
+	private _splatted: boolean;
+	//** Indicates if this node is only shown as its children, not itself */
+	get splatted(): boolean {
+		return this._splatted;
+	}
+	set splatted(value: boolean) {
+		if (this._splatted === value) return;
+
+		this._splatted = value;
+		// this.setId();
+	}
+
 	// NOTE: @eamodio uncomment to track node leaks
 	// readonly uuid = uuid();
 
@@ -262,8 +298,15 @@ export abstract class ViewNode<
 		// public readonly id: string | undefined,
 		uri: GitUri,
 		public readonly view: TView,
-		protected parent?: ViewNode,
+		protected parent?: ViewNode | undefined,
+		//** Indicates if this node is only shown as its children, not itself */
+		splatted?: boolean,
 	) {
+		this._splatted = splatted ?? false;
+		(parent ?? this).childrenCount++;
+
+		// this.setId();
+
 		// NOTE: @eamodio uncomment to track node leaks
 		// queueMicrotask(() => this.view.registerNode(this));
 		this._uri = uri;
@@ -277,9 +320,38 @@ export abstract class ViewNode<
 		// NOTE: @eamodio uncomment to track node leaks
 		// this.view.unregisterNode(this);
 	}
+	private _id!: string;
+	get id(): string {
+		if (this._id == null) {
+			// if (!this.splatted) {
+			this._id = this._uniqueId ?? `${(this.parent ?? this).childrenCount}:${this.type}`;
+			// }
+		}
+		return this._id;
+	}
+
+	get parentId(): string {
+		return this.parent?.treeId ?? '~';
+	}
+
+	get treeId(): string {
+		return this.splatted ? this.parentId : `${this.parentId}/${this.id}`;
+	}
 
-	get id(): string | undefined {
-		return this._uniqueId;
+	private setId() {
+		// if (this.splatted) {
+		// 	this._id = undefined!; //this.parent?.id ?? '~';
+		// } else {
+		// 	const { parent } = this;
+		// 	const { childrenIds } = parent ?? this;
+		// 	this._id = this._uniqueId ?? `${childrenIds.size ?? 0}:${this.type}`;
+		// 	if (childrenIds.has(this._id)) {
+		// 		debugger;
+		// 		// this._id = `${this._id}-${this._uniqueCounter++}`;
+		// 	}
+		// 	childrenIds.add(this._id);
+		// }
+		// console.log('#######', this.type, this.splatted, this._id);
 	}
 
 	private _context: AmbientContext | undefined;
@@ -339,32 +411,36 @@ export abstract class ViewNode<
 
 	getSplattedChild?(): Promise<ViewNode | undefined>;
 
+	protected get storedId(): string | undefined {
+		return this.id;
+	}
+
 	deleteState<T extends StateKey<State> = StateKey<State>>(key?: T): void {
-		if (this.id == null) {
+		if (this.storedId == null) {
 			debugger;
 			throw new Error('Id is required to delete state');
 		}
-		this.view.nodeState.deleteState(this.id, key as string);
+		this.view.nodeState.deleteState(this.storedId, key as string);
 	}
 
 	getState<T extends StateKey<State> = StateKey<State>>(key: T): StateValue<State, T> | undefined {
-		if (this.id == null) {
+		if (this.storedId == null) {
 			debugger;
 			throw new Error('Id is required to get state');
 		}
-		return this.view.nodeState.getState(this.id, key as string);
+		return this.view.nodeState.getState(this.storedId, key as string);
 	}
 
 	storeState<T extends StateKey<State> = StateKey<State>>(
 		key: T,
 		value: StateValue<State, T>,
 		sticky?: boolean,
 	): void {
-		if (this.id == null) {
+		if (this.storedId == null) {
 			debugger;
 			throw new Error('Id is required to store state');
 		}
-		this.view.nodeState.storeState(this.id, key as string, value, sticky);
+		this.view.nodeState.storeState(this.storedId, key as string, value, sticky);
 	}
 }
 

src/views/nodes/abstract/viewRefNode.ts

@@ -17,8 +17,9 @@ export abstract class ViewRefNode<
 		uri: GitUri,
 		view: TView,
 		protected override readonly parent: ViewNode,
+		splatted?: boolean,
 	) {
-		super(type, uri, view, parent);
+		super(type, uri, view, parent, splatted);
 	}
 
 	abstract get ref(): TReference;

src/views/nodes/autolinkedItemNode.ts

@@ -22,10 +22,6 @@ export class AutolinkedItemNode extends ViewNode<'autolink', ViewsWithCommits> {
 		this._uniqueId = getViewNodeId(`${this.type}+${item.id}`, this.context);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	override async toClipboard(type?: ClipboardType): Promise<string> {
 		const enriched = await this.maybeEnriched;
 		switch (type) {

src/views/nodes/autolinkedItemsNode.ts

@@ -30,10 +30,6 @@ export class AutolinkedItemsNode extends CacheableChildrenViewNode<'autolinks',
 		this._uniqueId = getViewNodeId(this.type, this.context);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	async getChildren(): Promise<ViewNode[]> {
 		if (this.children == null) {
 			const commits = [...this.log.commits.values()];

src/views/nodes/branchNode.ts

@@ -69,7 +69,6 @@ export class BranchNode
 	limit: number | undefined;
 
 	private readonly options: Options;
-	protected override splatted = true;
 
 	constructor(
 		uri: GitUri,
@@ -81,10 +80,12 @@ export class BranchNode
 		public readonly root: boolean,
 		options?: Partial<Options>,
 	) {
-		super('branch', uri, view, parent);
+		super('branch', uri, view, parent, root);
 
 		this.updateContext({ repository: repo, branch: branch, root: root });
-		this._uniqueId = getViewNodeId(this.type, this.context);
+		// this._uniqueId = getViewNodeId(this.type, this.context);
+		this._uniqueId = `${this.type}(${this.branch.id})`;
+
 		this.limit = this.view.getNodeLastKnownLimit(this);
 
 		this.options = {
@@ -107,10 +108,6 @@ export class BranchNode
 		this.children = undefined;
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	override toClipboard(): string {
 		return this.branch.name;
 	}
@@ -418,8 +415,6 @@ export class BranchNode
 	}
 
 	async getTreeItem(): Promise<TreeItem> {
-		this.splatted = false;
-
 		const parts = await getBranchNodeParts(this.view.container, this.branch, this.current, {
 			pendingPullRequest: this.getState('pendingPullRequest'),
 			showAsCommits: this.options.showAsCommits,

src/views/nodes/branchOrTagFolderNode.ts

@@ -22,10 +22,6 @@ export class BranchOrTagFolderNode extends ViewNode<'branch-tag-folder'> {
 		this._uniqueId = getViewNodeId(`${this.type}+${folderType}+${relativePath ?? folderName}`, this.context);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	override toClipboard(): string {
 		return this.folderName;
 	}

src/views/nodes/branchTrackingStatusNode.ts

@@ -56,14 +56,10 @@ export class BranchTrackingStatusNode
 			branchStatusUpstreamType: upstreamType,
 			root: root,
 		});
-		this._uniqueId = getViewNodeId(this.type, this.context);
+		// this._uniqueId = getViewNodeId(this.type, this.context);
 		this.limit = this.view.getNodeLastKnownLimit(this);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	get repoPath(): string {
 		return this.uri.repoPath!;
 	}

src/views/nodes/branchesNode.ts

@@ -19,16 +19,12 @@ export class BranchesNode extends CacheableChildrenViewNode<'branches', ViewsWit
 		protected override readonly parent: ViewNode,
 		public readonly repo: Repository,
 	) {
-		super('branches', uri, view, parent);
+		super('branches', uri, view, parent, true);
 
 		this.updateContext({ repository: repo });
 		this._uniqueId = getViewNodeId(this.type, this.context);
 	}
 
-	override get id(): string {
-		return this._uniqueId;
-	}
-
 	get repoPath(): string {
 		return this.repo.path;
 	}