diff --git a/src/components/Changes.svelte b/src/components/Changes.svelte index 585f9faff..91199e011 100644 --- a/src/components/Changes.svelte +++ b/src/components/Changes.svelte @@ -15,7 +15,12 @@ stepIndex, } from "@app/lib/commentNavigation"; import { diffOptions } from "@app/lib/diffOptions.svelte"; - import { fileDiffPath, fileMetaOf, fullFileLoader } from "@app/lib/diffText"; + import { + checkedPaths, + fileDiffPath, + fileMetaOf, + fullFileLoader, + } from "@app/lib/diffText"; import { draftReviewStorage } from "@app/lib/draftReviewStorage"; import { cachedDiffStats, @@ -207,28 +212,18 @@ const fileMeta = $derived(fileMetaOf(diffFiles)); const reviewedPaths = $derived.by(() => { - if (!draftReviewId) return undefined; - // eslint-disable-next-line svelte/prefer-svelte-reactivity -- rebuilt fresh each derivation - const paths = new Set(); - for (const file of diffFiles) { - const path = fileDiffPath(file); - if (draftReviewStorage.isFileChecked(draftReviewId, path)) { - paths.add(path); - } - } - return paths; + const id = draftReviewId; + if (!id) return undefined; + return checkedPaths(diffFiles, path => + draftReviewStorage.isFileChecked(id, path), + ); }); // Lockfiles and generated manifests are noise in a review, and a file already // marked reviewed has been dealt with, so both start collapsed. - const collapsedPaths = $derived.by(() => { - // eslint-disable-next-line svelte/prefer-svelte-reactivity -- rebuilt fresh each derivation - const paths = new Set(fileMeta.ignored); - for (const path of reviewedPaths ?? []) { - paths.add(path); - } - return paths; - }); + const collapsedPaths = $derived( + new Set([...fileMeta.ignored, ...(reviewedPaths ?? [])]), + ); const commentCounts = $derived.by(() => { const comments = diffCodeComments; diff --git a/src/lib/diffText.ts b/src/lib/diffText.ts index 2ea34dc5c..69910391e 100644 --- a/src/lib/diffText.ts +++ b/src/lib/diffText.ts @@ -130,3 +130,23 @@ export function fullFileLoader( return { oldContents, newContents }; }; } + +/// How many of the diff's files a reviewer has checked off. Checks left over +/// from files the diff no longer has don't count. +export function checkedFileProgress( + files: FileDiff[], + checkedFiles: string[], +): { filesChecked: number; filesTotal: number } { + const paths = new Set(files.map(fileDiffPath)); + return { + filesChecked: checkedFiles.filter(path => paths.has(path)).length, + filesTotal: files.length, + }; +} + +export function checkedPaths( + files: FileDiff[], + isChecked: (path: string) => boolean, +): Set { + return new Set(files.map(fileDiffPath).filter(isChecked)); +} diff --git a/src/views/repo/Patch.svelte b/src/views/repo/Patch.svelte index b5449b9f4..561f9e407 100644 --- a/src/views/repo/Patch.svelte +++ b/src/views/repo/Patch.svelte @@ -12,10 +12,12 @@ import type { Commit } from "@bindings/repo/Commit"; import type { RepoInfo } from "@bindings/repo/RepoInfo"; + import countBy from "lodash/countBy"; import mapValues from "lodash/mapValues"; import { SvelteSet } from "svelte/reactivity"; import { commentSourcesOf } from "@app/lib/commentSources"; + import { checkedFileProgress } from "@app/lib/diffText"; import { draftReviewStorage } from "@app/lib/draftReviewStorage"; import { nodeRunning } from "@app/lib/events"; import { @@ -329,16 +331,8 @@ status: undefined, }), ]); - const patchCounts: Record = {}; - for (const p of patches.content) { - patchCounts[p.author.did] = (patchCounts[p.author.did] ?? 0) + 1; - } - const issueCounts: Record = {}; - for (const i of issues) { - issueCounts[i.author.did] = (issueCounts[i.author.did] ?? 0) + 1; - } - patchesAuthoredByDid = patchCounts; - issuesAuthoredByDid = issueCounts; + patchesAuthoredByDid = countBy(patches.content, p => p.author.did); + issuesAuthoredByDid = countBy(issues, i => i.author.did); lastActivityRid = targetRid; activityLoaded = true; } catch (error) { @@ -554,18 +548,7 @@ head: rev.head, }).then(diff => { if (cancelled) return; - const filePaths = new Set( - diff.files.map(f => - f.status === "moved" || f.status === "copied" ? f.newPath : f.path, - ), - ); - const filesChecked = draft.checkedFiles.filter(p => - filePaths.has(p), - ).length; - fileProgress = { - filesChecked, - filesTotal: diff.files.length, - }; + fileProgress = checkedFileProgress(diff.files, draft.checkedFiles); }); return () => { cancelled = true;