From e93149d9a8f1bfa074778853adccd74858a0e55d Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Mon, 1 Apr 2024 15:17:20 +0200 Subject: [PATCH] Refactor `onwership.ts` - replaces a set with a map so we can use hunk hash in api calls - shortens method names, e.g. `addHunk` -> `add` --- .../lib/components/BranchFilesHeader.svelte | 10 +-- .../src/lib/components/CommitDialog.svelte | 2 +- .../src/lib/components/FileListItem.svelte | 8 +- .../src/lib/components/FileTree.svelte | 4 +- .../src/lib/components/HunkViewer.svelte | 6 +- .../src/lib/components/TreeListFile.svelte | 10 +-- .../src/lib/components/TreeListFolder.svelte | 13 +-- gitbutler-ui/src/lib/vbranches/ownership.ts | 87 +++++++++++-------- 8 files changed, 79 insertions(+), 61 deletions(-) diff --git a/gitbutler-ui/src/lib/components/BranchFilesHeader.svelte b/gitbutler-ui/src/lib/components/BranchFilesHeader.svelte index 392691cbc..0af405330 100644 --- a/gitbutler-ui/src/lib/components/BranchFilesHeader.svelte +++ b/gitbutler-ui/src/lib/components/BranchFilesHeader.svelte @@ -16,14 +16,12 @@ function selectAll(files: AnyFile[]) { if (!selectedOwnership) return; - files.forEach((f) => - selectedOwnership.update((ownership) => ownership.addHunk(f.id, ...f.hunks.map((h) => h.id))) - ); + files.forEach((f) => selectedOwnership.update((ownership) => ownership.add(f.id, ...f.hunks))); } function isAllChecked(selectedOwnership: Ownership | undefined): boolean { if (!selectedOwnership) return false; - return files.every((f) => f.hunks.every((h) => selectedOwnership.containsHunk(f.id, h.id))); + return files.every((f) => f.hunks.every((h) => selectedOwnership.contains(f.id, h.id))); } function isIndeterminate(selectedOwnership: Ownership | undefined): boolean { @@ -31,10 +29,10 @@ if (files.length <= 1) return false; let file = files[0]; - let prev = selectedOwnership.containsHunk(file.id, ...file.hunkIds); + let prev = selectedOwnership.contains(file.id, ...file.hunkIds); for (let i = 1; i < files.length; i++) { file = files[i]; - const contained = selectedOwnership.containsHunk(file.id, ...file.hunkIds); + const contained = selectedOwnership.contains(file.id, ...file.hunkIds); if (contained != prev) { return true; } diff --git a/gitbutler-ui/src/lib/components/CommitDialog.svelte b/gitbutler-ui/src/lib/components/CommitDialog.svelte index 9780a6bd1..a6a01259f 100644 --- a/gitbutler-ui/src/lib/components/CommitDialog.svelte +++ b/gitbutler-ui/src/lib/components/CommitDialog.svelte @@ -91,7 +91,7 @@ async function generateCommitMessage(files: LocalFile[]) { const hunks = files.flatMap((f) => - f.hunks.filter((h) => $selectedOwnership.containsHunk(f.id, h.id)) + f.hunks.filter((h) => $selectedOwnership.contains(f.id, h.id)) ); // Branches get their names generated only if there are at least 4 lines of code // If the change is a 'one-liner', the branch name is either left as "virtual branch" diff --git a/gitbutler-ui/src/lib/components/FileListItem.svelte b/gitbutler-ui/src/lib/components/FileListItem.svelte index 66acf4507..5903b2822 100644 --- a/gitbutler-ui/src/lib/components/FileListItem.svelte +++ b/gitbutler-ui/src/lib/components/FileListItem.svelte @@ -28,9 +28,9 @@ $: if (file && $selectedOwnership) { const fileId = file.id; - checked = file.hunks.every((hunk) => $selectedOwnership?.containsHunk(fileId, hunk.id)); + checked = file.hunks.every((hunk) => $selectedOwnership?.contains(fileId, hunk.id)); const selectedCount = file.hunks.filter((hunk) => - $selectedOwnership?.containsHunk(fileId, hunk.id) + $selectedOwnership?.contains(fileId, hunk.id) ).length; indeterminate = selectedCount > 0 && file.hunks.length - selectedCount > 0; } @@ -61,8 +61,8 @@ {indeterminate} on:change={(e) => { selectedOwnership?.update((ownership) => { - if (e.detail) file.hunks.forEach((h) => ownership.addHunk(file.id, h.id)); - if (!e.detail) file.hunks.forEach((h) => ownership.removeHunk(file.id, h.id)); + if (e.detail) file.hunks.forEach((h) => ownership.add(file.id, h)); + if (!e.detail) file.hunks.forEach((h) => ownership.remove(file.id, h.id)); return ownership; }); }} diff --git a/gitbutler-ui/src/lib/components/FileTree.svelte b/gitbutler-ui/src/lib/components/FileTree.svelte index f63a02d56..28676de36 100644 --- a/gitbutler-ui/src/lib/components/FileTree.svelte +++ b/gitbutler-ui/src/lib/components/FileTree.svelte @@ -27,7 +27,7 @@ function isNodeChecked(selectedOwnership: Ownership, node: TreeNode): boolean { if (node.file) { const fileId = node.file.id; - return node.file.hunks.some((hunk) => selectedOwnership.containsHunk(fileId, hunk.id)); + return node.file.hunks.some((hunk) => selectedOwnership.contains(fileId, hunk.id)); } else { return node.children.every((child) => isNodeChecked(selectedOwnership, child)); } @@ -39,7 +39,7 @@ if (node.file) { const fileId = node.file.id; const numSelected = node.file.hunks.filter( - (hunk) => !selectedOwnership.containsHunk(fileId, hunk.id) + (hunk) => !selectedOwnership.contains(fileId, hunk.id) ).length; return numSelected !== node.file.hunks.length && numSelected !== 0; } diff --git a/gitbutler-ui/src/lib/components/HunkViewer.svelte b/gitbutler-ui/src/lib/components/HunkViewer.svelte index 245622020..8990bc2c4 100644 --- a/gitbutler-ui/src/lib/components/HunkViewer.svelte +++ b/gitbutler-ui/src/lib/components/HunkViewer.svelte @@ -33,9 +33,9 @@ function onHunkSelected(hunk: Hunk, isSelected: boolean) { if (!selectedOwnership) return; if (isSelected) { - selectedOwnership.update((ownership) => ownership.addHunk(hunk.filePath, hunk.id)); + selectedOwnership.update((ownership) => ownership.add(hunk.filePath, hunk)); } else { - selectedOwnership.update((ownership) => ownership.removeHunk(hunk.filePath, hunk.id)); + selectedOwnership.update((ownership) => ownership.remove(hunk.filePath, hunk.id)); } } @@ -93,7 +93,7 @@ {selectable} {draggingDisabled} tabSize={$userSettings.tabSize} - selected={$selectedOwnership?.containsHunk(hunk.filePath, hunk.id)} + selected={$selectedOwnership?.contains(hunk.filePath, hunk.id)} on:selected={(e) => onHunkSelected(hunk, e.detail)} sectionType={subsection.sectionType} on:contextmenu={(e) => diff --git a/gitbutler-ui/src/lib/components/TreeListFile.svelte b/gitbutler-ui/src/lib/components/TreeListFile.svelte index 2457f4994..694d0a06c 100644 --- a/gitbutler-ui/src/lib/components/TreeListFile.svelte +++ b/gitbutler-ui/src/lib/components/TreeListFile.svelte @@ -31,10 +31,8 @@ function updateOwnership(ownership: Ownership | undefined) { if (!ownership) return; const fileId = file.id; - checked = file.hunks.every((hunk) => ownership.containsHunk(fileId, hunk.id)); - const selectedCount = file.hunks.filter((hunk) => - ownership.containsHunk(fileId, hunk.id) - ).length; + checked = file.hunks.every((hunk) => ownership.contains(fileId, hunk.id)); + const selectedCount = file.hunks.filter((hunk) => ownership.contains(fileId, hunk.id)).length; indeterminate = selectedCount > 0 && file.hunks.length - selectedCount > 0; if (indeterminate) checked = false; } @@ -89,8 +87,8 @@ {indeterminate} on:change={(e) => { selectedOwnership?.update((ownership) => { - if (e.detail) file.hunks.forEach((h) => ownership.addHunk(file.id, h.id)); - if (!e.detail) file.hunks.forEach((h) => ownership.removeHunk(file.id, h.id)); + if (e.detail) file.hunks.forEach((h) => ownership.add(file.id, h)); + if (!e.detail) file.hunks.forEach((h) => ownership.remove(file.id, h.id)); return ownership; }); }} diff --git a/gitbutler-ui/src/lib/components/TreeListFolder.svelte b/gitbutler-ui/src/lib/components/TreeListFolder.svelte index 084cc7187..3f097d0b8 100644 --- a/gitbutler-ui/src/lib/components/TreeListFolder.svelte +++ b/gitbutler-ui/src/lib/components/TreeListFolder.svelte @@ -4,6 +4,7 @@ import { maybeGetContextStore } from '$lib/utils/context'; import { Ownership } from '$lib/vbranches/ownership'; import type { TreeNode } from '$lib/vbranches/filetree'; + import type { Hunk, RemoteHunk } from '$lib/vbranches/types'; import type { Writable } from 'svelte/store'; export let expanded: boolean; @@ -14,19 +15,21 @@ const selectedOwnership: Writable | undefined = maybeGetContextStore(Ownership); - function idWithChildren(node: TreeNode): [string, string[]][] { + function idWithChildren(node: TreeNode): [string, (Hunk | RemoteHunk)[]][] { if (node.file) { - return [[node.file.id, node.file.hunks.map((h) => h.id)]]; + return [[node.file.id, node.file.hunks]]; } return node.children.flatMap(idWithChildren); } function onSelectionChanged() { - idWithChildren(node).forEach(([fileId, hunkIds]) => { + idWithChildren(node).forEach(([fileId, hunks]) => { if (isChecked) { - selectedOwnership?.update((ownership) => ownership.removeHunk(fileId, ...hunkIds)); + selectedOwnership?.update((ownership) => + ownership.remove(fileId, ...hunks.map((h) => h.id)) + ); } else { - selectedOwnership?.update((ownership) => ownership.addHunk(fileId, ...hunkIds)); + selectedOwnership?.update((ownership) => ownership.add(fileId, ...hunks)); } }); } diff --git a/gitbutler-ui/src/lib/vbranches/ownership.ts b/gitbutler-ui/src/lib/vbranches/ownership.ts index b2f152e51..232932d12 100644 --- a/gitbutler-ui/src/lib/vbranches/ownership.ts +++ b/gitbutler-ui/src/lib/vbranches/ownership.ts @@ -1,4 +1,4 @@ -import type { Branch, AnyFile } from './types'; +import type { Branch, AnyFile, Hunk, RemoteHunk } from './types'; export function filesToOwnership(files: AnyFile[]) { return files @@ -6,71 +6,90 @@ export function filesToOwnership(files: AnyFile[]) { .join('\n'); } -export class Ownership { - files: Map>; +// These types help keep track of what maps to what. +// TODO: refactor code for clarity, these types should not be needed +export type AnyHunk = Hunk | RemoteHunk; +export type HunkId = string; +export type FilePath = string; +export type HunkClaims = Map; +export type FileClaims = Map; - static default() { - return new Ownership(new Map()); - } +export class Ownership { + private claims: FileClaims; static fromBranch(branch: Branch) { const files = branch.files.reduce((acc, file) => { - if (acc.has(file.id)) { - const existing = acc.get(file.id); - file.hunks.forEach((hunk) => existing.add(hunk.id)); + const existing = acc.get(file.id); + if (existing) { + file.hunks.forEach((hunk) => existing.set(hunk.id, hunk)); } else { acc.set( file.id, - file.hunks.reduce((acc, hunk) => { - acc.add(hunk.id); - return acc; - }, new Set()) + file.hunks.reduce((acc2, hunk) => { + return acc2.set(hunk.id, hunk); + }, new Map()) ); } return acc; - }, new Map()); - return new Ownership(files); + }, new Map>()); + const ownership = new Ownership(files); + return ownership; } - constructor(files: Map>) { - this.files = files; + constructor(files: FileClaims) { + this.claims = files; } - removeHunk(fileId: string, ...hunkIds: string[]) { - const hunks = this.files.get(fileId); - if (hunks) { - hunkIds.forEach((hunkId) => hunks.delete(hunkId)); - if (hunks.size === 0) this.files.delete(fileId); - } + remove(fileId: string, ...hunkIds: string[]) { + const claims = this.claims; + if (!claims) return this; + hunkIds.forEach((hunkId) => { + claims.get(fileId)?.delete(hunkId); + if (claims.get(fileId)?.size == 0) claims.delete(fileId); + }); return this; } - addHunk(fileId: string, ...hunkIds: string[]) { - const hunks = this.files.get(fileId); - if (hunks) { - hunkIds.forEach((hunkId) => hunks.add(hunkId)); + add(fileId: string, ...items: AnyHunk[]) { + const claim = this.claims.get(fileId); + if (claim) { + items.forEach((hunk) => claim.set(hunk.id, hunk)); } else { - this.files.set(fileId, new Set(hunkIds)); + this.claims.set( + fileId, + items.reduce((acc, hunk) => { + return acc.set(hunk.id, hunk); + }, new Map()) + ); } return this; } - containsHunk(fileId: string, ...hunkIds: string[]): boolean { - return hunkIds.every((hunkId) => !!this.files.get(fileId)?.has(hunkId)); + contains(fileId: string, ...hunkIds: string[]): boolean { + return hunkIds.every((hunkId) => !!this.claims.get(fileId)?.has(hunkId)); } clear() { - this.files.clear(); + this.claims.clear(); return this; } toString() { - return Array.from(this.files.entries()) - .map(([fileId, hunks]) => fileId + ':' + Array.from(hunks.values()).join(',')) + return Array.from(this.claims.entries()) + .map( + ([fileId, hunkMap]) => + fileId + + ':' + + Array.from(hunkMap.values()) + .map((hunk) => { + return `${hunk.id}-${hunk.hash}`; + }) + .join(',') + ) .join('\n'); } isEmpty() { - return this.files.size == 0; + return this.claims.size == 0; } }