From 832a21e25f750d4985dbba4d0da405827a12be5f Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 28 Sep 2025 14:08:03 +0100 Subject: [PATCH 01/13] Remove unnecessary runtime parameter The webView already uses SkeletonElementIds and we never use a different payload.diffContainer value anyway. --- src/extension/provider.ts | 2 -- src/webview/message/api.ts | 1 - src/webview/message/handler.ts | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/extension/provider.ts b/src/extension/provider.ts index d9bfa06..4899b9a 100644 --- a/src/extension/provider.ts +++ b/src/extension/provider.ts @@ -2,7 +2,6 @@ import { parse } from "diff2html"; import { ColorSchemeType } from "diff2html/lib/types"; import { basename } from "path"; import * as vscode from "vscode"; -import { SkeletonElementIds } from "../shared/css/elements"; import { MessageToExtensionHandler, MessageToWebview } from "../shared/message"; import { APP_CONFIG_SECTION, AppConfig, extractConfig, setOutputFormatConfig } from "./configuration"; import { MessageToExtensionHandlerImpl } from "./message/handler"; @@ -154,7 +153,6 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { config: config, diffFiles: diffFiles, viewedState: viewedState, - diffContainer: SkeletonElementIds.DiffContainer, }, }, }); diff --git a/src/webview/message/api.ts b/src/webview/message/api.ts index b91c771..2514ebc 100644 --- a/src/webview/message/api.ts +++ b/src/webview/message/api.ts @@ -5,7 +5,6 @@ import { ViewedState } from "../../extension/viewed-state"; export interface UpdateWebviewPayload { config: AppConfig; diffFiles: DiffFile[]; - diffContainer: string; viewedState: ViewedState; } diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index 7ebda01..35a1f29 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -31,7 +31,7 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple } public async updateWebview(payload: UpdateWebviewPayload): Promise { - const diffContainer = document.getElementById(payload.diffContainer); + const diffContainer = document.getElementById(SkeletonElementIds.DiffContainer); if (!diffContainer) { return; } From 5a00da78811e457ec18aa6475bc650d17f055a38 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 28 Sep 2025 14:11:41 +0100 Subject: [PATCH 02/13] Renaming for clarity isVisible is a bit ambiguous - we're making the main body *not* visible and the loading messages visible. Then a consistent renaming for showEmpty(). --- src/webview/message/handler.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index 35a1f29..10127e6 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -310,21 +310,21 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple this.showLoading(false); } - private showLoading(isVisible: boolean): void { + private showLoading(isLoading: boolean): void { const loadingContainer = document.getElementById(SkeletonElementIds.LoadingContainer); if (!loadingContainer) { return; } - loadingContainer.style.display = isVisible ? "block" : "none"; + loadingContainer.style.display = isLoading ? "block" : "none"; } - private showEmpty(isVisible: boolean): void { + private showEmpty(isEmpty: boolean): void { const emptyMessageContainer = document.getElementById(SkeletonElementIds.EmptyMessageContainer); if (!emptyMessageContainer) { return; } - emptyMessageContainer.style.display = isVisible ? "block" : "none"; + emptyMessageContainer.style.display = isEmpty ? "block" : "none"; } } From 7b95b1a0c399ff0bcac8fc8933c952d42352f814 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 28 Sep 2025 14:13:04 +0100 Subject: [PATCH 03/13] Remove state objects when empty This makes the workspace storage file a bit smaller. --- src/extension/viewed-state.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/extension/viewed-state.ts b/src/extension/viewed-state.ts index 7349b4e..349794e 100644 --- a/src/extension/viewed-state.ts +++ b/src/extension/viewed-state.ts @@ -34,7 +34,9 @@ export class ViewedStateStore { if (!this.args.docId) { this.transientViewedState = viewedState; } else { - this.args.context.workspaceState.update(this.args.docId, viewedState); + // remove the state if no files are marked as viewed + const stateToSave = Object.keys(viewedState).length === 0 ? undefined : viewedState; + this.args.context.workspaceState.update(this.args.docId, stateToSave); } } } From 47d57b4cafec9e2ce06eb64131d85e84c96474e3 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sun, 28 Sep 2025 14:24:00 +0100 Subject: [PATCH 04/13] Fix: Collapse viewed files without scrolling When we use toggle.click(), that toggle gets focused, which causes the window to scroll to the last collapsed file when we switch to that tab. That's disruptive and this fixes it. --- src/webview/css/classes.ts | 4 ++++ src/webview/css/elements.ts | 1 + src/webview/message/handler.ts | 13 ++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 src/webview/css/classes.ts diff --git a/src/webview/css/classes.ts b/src/webview/css/classes.ts new file mode 100644 index 0000000..520ad6b --- /dev/null +++ b/src/webview/css/classes.ts @@ -0,0 +1,4 @@ +export enum Diff2HtmlCssClasses { + Div__DiffFileContent__Collapsed = "d2h-d-none", + Input__ViewedToggle__Selected = "d2h-selected", +} diff --git a/src/webview/css/elements.ts b/src/webview/css/elements.ts index c50311f..bb14ded 100644 --- a/src/webview/css/elements.ts +++ b/src/webview/css/elements.ts @@ -5,6 +5,7 @@ export enum Diff2HtmlCssClassElements { Div__File = ".d2h-file-wrapper", Div__LeftDiffOnSideBySide__FirstChild = ".d2h-file-side-diff:first-child", Div__LineNumberRightOnLineByLine = ".line-num2", + Label__ViewedToggle = ".d2h-file-collapse", Input__ViewedToggle = ".d2h-file-collapse-input", Input__ViewedToggle__Checked = ".d2h-file-collapse-input:checked", Td__DeletedLine = ".d2h-del", diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index 10127e6..fe47c65 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -7,6 +7,7 @@ import { extractNewFileNameFromDiffName, extractNumberFromString } from "../../s import { MessageToExtension, MessageToWebviewHandler } from "../../shared/message"; import { GenericMessageHandlerImpl } from "../../shared/message-handler"; import { AppTheme } from "../../shared/types"; +import { Diff2HtmlCssClasses } from "../css/classes"; import { Diff2HtmlCssClassElements } from "../css/elements"; import { UpdateWebviewPayload } from "./api"; import { getSha1Hash } from "./hash"; @@ -270,7 +271,7 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple if (fileName && viewedState[fileName]) { const diffHash = await this.getDiffHash(toggle); if (diffHash === viewedState[fileName]) { - toggle.click(); + this.updateDiff2HtmlFileCollapsed(toggle, true); } else { toggle.classList.add(CHANGED_SINCE_VIEWED); } @@ -278,6 +279,16 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple } } + private updateDiff2HtmlFileCollapsed(toggleElement: HTMLInputElement, collapse: boolean) { + // do the same thing that diff2html does to collapse or expand one file + toggleElement.checked = collapse; + const fileContainer = this.getDiffFileContainer(toggleElement); + const label = fileContainer?.querySelector(Diff2HtmlCssClassElements.Label__ViewedToggle); + label?.classList.toggle(Diff2HtmlCssClasses.Input__ViewedToggle__Selected, collapse); + const fileContent = fileContainer?.querySelector(Diff2HtmlCssClassElements.Div__DiffFileContent); + fileContent?.classList.toggle(Diff2HtmlCssClasses.Div__DiffFileContent__Collapsed, collapse); + } + private async getDiffHash(diffElement: HTMLElement): Promise { const fileContainer = this.getDiffFileContainer(diffElement); const fileContent = fileContainer?.querySelector(Diff2HtmlCssClassElements.Div__DiffFileContent)?.innerHTML; From 377efe7e711b63aa72b799d7ed4bfcf14ff616c3 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 16:48:54 +0100 Subject: [PATCH 05/13] Add diffviewer.openCollapsed command This command is in the context menu for diff files in the explorer. It opens the diff file with all parts collapsed (marked as viewed) to potentially make it easier to deal with very large diff files. --- package.json | 11 +++++++ src/extension/provider.ts | 11 +++++-- src/webview/message/api.ts | 1 + src/webview/message/handler.ts | 53 +++++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index aca82f7..504c6ed 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,10 @@ "command": "diffviewer.showSideBySide", "title": "Show diff side by side", "icon": "$(files)" + }, + { + "command": "diffviewer.openCollapsed", + "title": "Open diff collapsed (all viewed)" } ], "customEditors": [ @@ -142,6 +146,13 @@ } ], "menus": { + "explorer/context": [ + { + "command": "diffviewer.openCollapsed", + "group": "navigation@99", + "when": "resourceLangId == 'diff'" + } + ], "editor/title": [ { "command": "diffviewer.showLineByLine", diff --git a/src/extension/provider.ts b/src/extension/provider.ts index 4899b9a..beb8dd1 100644 --- a/src/extension/provider.ts +++ b/src/extension/provider.ts @@ -43,6 +43,10 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { }), vscode.commands.registerCommand("diffviewer.showLineByLine", () => setOutputFormatConfig("line-by-line")), vscode.commands.registerCommand("diffviewer.showSideBySide", () => setOutputFormatConfig("side-by-side")), + vscode.commands.registerCommand("diffviewer.openCollapsed", async (file) => { + const collapsedUri = vscode.Uri.from({ ...file, query: "collapsed" }); + await vscode.commands.executeCommand("vscode.openWith", collapsedUri, "diffViewer"); + }), ]; } @@ -71,6 +75,8 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { docId: diffDocument.uri.fsPath, }); + const collapseAll = new URLSearchParams(diffDocument.uri.query).has("collapsed"); + const messageReceivedHandler = new MessageToExtensionHandlerImpl({ diffDocument, viewedStateStore, @@ -82,7 +88,7 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { const webviewContext: WebviewContext = { document: diffDocument, panel: webviewPanel, viewedStateStore }; this.registerEventHandlers({ webviewContext, messageHandler: messageReceivedHandler }); - this.updateWebview(webviewContext); + this.updateWebview(webviewContext, collapseAll); } private registerEventHandlers(args: { webviewContext: WebviewContext; messageHandler: MessageToExtensionHandler }) { @@ -112,7 +118,7 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { args.webviewContext.panel.onDidDispose(() => disposables.dispose()); } - private updateWebview(webviewContext: WebviewContext): void { + private updateWebview(webviewContext: WebviewContext, collapseAll = false): void { this.postMessageToWebviewWrapper({ webview: webviewContext.panel.webview, message: { @@ -153,6 +159,7 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { config: config, diffFiles: diffFiles, viewedState: viewedState, + collapseAll, }, }, }); diff --git a/src/webview/message/api.ts b/src/webview/message/api.ts index 2514ebc..8c9ace8 100644 --- a/src/webview/message/api.ts +++ b/src/webview/message/api.ts @@ -6,6 +6,7 @@ export interface UpdateWebviewPayload { config: AppConfig; diffFiles: DiffFile[]; viewedState: ViewedState; + collapseAll: boolean; } export interface MessageToWebviewApi { diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index fe47c65..3f7f67f 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -47,13 +47,25 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple const appTheme = this.currentConfig.diff2html.colorScheme === ColorSchemeType.DARK ? "dark" : "light"; this.setupTheme(appTheme); + // hide the diff container for now so the full diff doesn't show + if (payload.collapseAll) { + diffContainer.style.display = "none"; + } + const diff2html = new Diff2HtmlUI(diffContainer, payload.diffFiles, this.currentConfig.diff2html); diff2html.draw(); this.registerViewedToggleHandlers(diffContainer); this.registerDiffContainerHandlers(diffContainer); - await this.hideViewedFiles(diffContainer, payload.viewedState); + if (payload.collapseAll) { + this.setAllViewedStates(true); + } else { + await this.hideViewedFiles(diffContainer, payload.viewedState); + } this.updateFooter(); + + // show the diff + diffContainer.style.display = "block"; }); } @@ -92,7 +104,7 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple viewedToggle.classList.remove(CHANGED_SINCE_VIEWED); this.scrollDiffFileHeaderIntoView(viewedToggle); this.updateFooter(); - this.sendFileViewedMessage(viewedToggle); + this.sendFileViewedMessage(viewedToggle, viewedToggle.checked); } private onMarkAllViewedChangedHandler(event: Event): void { @@ -102,12 +114,17 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple } const isChecked = markAllViewedCheckbox.checked; + this.setAllViewedStates(isChecked); + this.updateFooter(); + } + + private setAllViewedStates(viewed: boolean) { const allToggles = this.getViewedToggles(); for (const toggle of Array.from(allToggles)) { - if (toggle.checked !== isChecked) { - toggle.click(); - } + this.updateDiff2HtmlFileCollapsed(toggle, viewed); + // no await here so we synchronously return from this function, the sending will happen later as hashes get computed + this.sendFileViewedMessage(toggle, viewed); } } @@ -266,15 +283,23 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple const viewedToggles = diffContainer.querySelectorAll( Diff2HtmlCssClassElements.Input__ViewedToggle, ); + + // first synchronously hide all viewed files so we don't render too much in case the file is huge and most is collapsed + // we'll do a second pass over hidden files, so we'll hold them in togglesToRevisit + const togglesToRevisit = []; for (const toggle of Array.from(viewedToggles)) { const fileName = this.getDiffElementFileName(toggle); if (fileName && viewedState[fileName]) { - const diffHash = await this.getDiffHash(toggle); - if (diffHash === viewedState[fileName]) { - this.updateDiff2HtmlFileCollapsed(toggle, true); - } else { - toggle.classList.add(CHANGED_SINCE_VIEWED); - } + togglesToRevisit.push({ toggle, oldSha1: viewedState[fileName] }); + this.updateDiff2HtmlFileCollapsed(toggle, true); + } + } + + for (const { toggle, oldSha1 } of togglesToRevisit) { + const diffHash = await this.getDiffHash(toggle); + if (diffHash !== oldSha1) { + this.updateDiff2HtmlFileCollapsed(toggle, false); + toggle.classList.add(CHANGED_SINCE_VIEWED); } } } @@ -295,13 +320,13 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple return fileContent ? getSha1Hash(fileContent) : null; } - private async sendFileViewedMessage(toggleElement: HTMLInputElement): Promise { - const fileName = this.getDiffElementFileName(toggleElement); + private async sendFileViewedMessage(diffElement: HTMLInputElement, viewed: boolean): Promise { + const fileName = this.getDiffElementFileName(diffElement); if (!fileName) { return; } - const viewedSha1 = toggleElement.checked ? await this.getDiffHash(toggleElement) : null; + const viewedSha1 = viewed ? await this.getDiffHash(diffElement) : null; this.postMessageToExtensionFn({ kind: "toggleFileViewed", From 7394e878905822fe58248b17304bbd4ce740c8da Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 21:42:13 +0100 Subject: [PATCH 06/13] Make tests pass --- src/extension/__tests__/provider.test.ts | 7 ++- src/webview/message/__tests__/handler.test.ts | 43 +++++++++++++------ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/extension/__tests__/provider.test.ts b/src/extension/__tests__/provider.test.ts index e67f715..19ca0cc 100644 --- a/src/extension/__tests__/provider.test.ts +++ b/src/extension/__tests__/provider.test.ts @@ -7,7 +7,6 @@ import { MessageToExtensionHandlerImpl } from "../message/handler"; import { ViewedStateStore } from "../viewed-state"; import { buildSkeleton } from "../skeleton"; import { extractConfig, setOutputFormatConfig } from "../configuration"; -import { SkeletonElementIds } from "../../shared/css/elements"; import { MessageToWebview } from "../../shared/message"; // Mock dependencies @@ -188,7 +187,7 @@ describe("DiffViewerProvider", () => { webviewPath: mockWebviewPath, }); - expect(disposables).toHaveLength(3); + expect(disposables).toHaveLength(4); expect(mockRegisterCustomEditorProvider).toHaveBeenCalledWith("diffViewer", expect.any(DiffViewerProvider), { webviewOptions: { retainContextWhenHidden: true, @@ -198,6 +197,7 @@ describe("DiffViewerProvider", () => { }); expect(mockRegisterCommand).toHaveBeenCalledWith("diffviewer.showLineByLine", expect.any(Function)); expect(mockRegisterCommand).toHaveBeenCalledWith("diffviewer.showSideBySide", expect.any(Function)); + expect(mockRegisterCommand).toHaveBeenCalledWith("diffviewer.openCollapsed", expect.any(Function)); }); it("should call setOutputFormatConfig when commands are executed", () => { @@ -307,7 +307,7 @@ describe("DiffViewerProvider", () => { config: expect.any(Object), diffFiles: expect.any(Array), viewedState: expect.any(Object), - diffContainer: SkeletonElementIds.DiffContainer, + collapseAll: false, }, }); }); @@ -452,7 +452,6 @@ describe("DiffViewerProvider", () => { config: expect.any(Object), diffFiles: expect.any(Array), viewedState: expect.any(Object), - diffContainer: SkeletonElementIds.DiffContainer, }), }); }); diff --git a/src/webview/message/__tests__/handler.test.ts b/src/webview/message/__tests__/handler.test.ts index f08e299..f359f9b 100644 --- a/src/webview/message/__tests__/handler.test.ts +++ b/src/webview/message/__tests__/handler.test.ts @@ -203,6 +203,7 @@ describe("MessageToWebviewHandlerImpl", () => { mockDiffContainer = { querySelectorAll: mockQuerySelectorAll, addEventListener: mockAddEventListener, + style: {}, } as unknown as HTMLElement; mockDiffFiles = [createMockDiffFile("file1.ts"), createMockDiffFile("file2.ts")]; @@ -216,8 +217,8 @@ describe("MessageToWebviewHandlerImpl", () => { await handler.updateWebview({ config: mockConfig, diffFiles: mockDiffFiles, - diffContainer: "test-container", viewedState: mockViewedState, + collapseAll: false, }); expect(Diff2HtmlUI).not.toHaveBeenCalled(); @@ -227,12 +228,13 @@ describe("MessageToWebviewHandlerImpl", () => { const mockDiffContainer = { querySelectorAll: mockQuerySelectorAll, addEventListener: mockAddEventListener, + style: {}, } as unknown as HTMLElement; // Reset the mock to use the global implementation mockGetElementById.mockReset(); mockGetElementById.mockImplementation((id: string) => { - if (id === "test-container") { + if (id === "diff-container") { return mockDiffContainer; } if (id === "mark-all-viewed-checkbox") { @@ -259,8 +261,8 @@ describe("MessageToWebviewHandlerImpl", () => { await handler.updateWebview({ config: mockConfig, diffFiles: [], - diffContainer: "test-container", viewedState: mockViewedState, + collapseAll: false, }); expect(mockGetElementById).toHaveBeenCalledWith("empty-message-container"); @@ -278,7 +280,7 @@ describe("MessageToWebviewHandlerImpl", () => { // Reset the mock to use the global implementation mockGetElementById.mockReset(); mockGetElementById.mockImplementation((id: string) => { - if (id === "test-container") { + if (id === "diff-container") { return mockDiffContainer; } if (id === "mark-all-viewed-checkbox") { @@ -309,8 +311,8 @@ describe("MessageToWebviewHandlerImpl", () => { await handler.updateWebview({ config: mockConfig, diffFiles: mockDiffFiles, - diffContainer: "test-container", viewedState: mockViewedState, + collapseAll: false, }); expect(Diff2HtmlUI).toHaveBeenCalledWith(mockDiffContainer, mockDiffFiles, mockConfig.diff2html); @@ -376,22 +378,28 @@ describe("MessageToWebviewHandlerImpl", () => { addEventListener: mockAddEventListener, } as unknown as HTMLInputElement; + const mockClosest = () => ({ querySelector: () => ({ classList: { toggle: jest.fn() } }) }); + const mockToggles = [ - { checked: false, click: mockClick }, - { checked: false, click: mockClick }, + { checked: false, closest: mockClosest }, + { checked: false, closest: mockClosest }, ]; mockGetElementById.mockReturnValue(mockMarkAllCheckbox); // eslint-disable-next-line @typescript-eslint/no-explicit-any (handler as any).getViewedToggles = jest.fn().mockReturnValue(mockToggles); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (handler as any).updateFooter = jest.fn(); const mockEvent = { target: mockMarkAllCheckbox } as unknown as Event; // eslint-disable-next-line @typescript-eslint/no-explicit-any (handler as any).onMarkAllViewedChangedHandler(mockEvent); - expect(mockToggles[0].click).toHaveBeenCalled(); - expect(mockToggles[1].click).toHaveBeenCalled(); + expect(mockToggles[0].checked).toBe(true); + expect(mockToggles[1].checked).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((handler as any).updateFooter).toHaveBeenCalled(); }); }); @@ -560,6 +568,7 @@ describe("MessageToWebviewHandlerImpl", () => { click: mockClick, classList: { add: jest.fn() }, closest: mockClosest, + checked: false, // this will be set to true if the toggle is switched }; const mockFileContainer = { @@ -568,10 +577,11 @@ describe("MessageToWebviewHandlerImpl", () => { const mockDiffContainer = { querySelectorAll: jest.fn().mockReturnValue([mockToggle]), + style: {}, } as unknown as HTMLElement; mockClosest.mockReturnValue(mockFileContainer); - mockQuerySelector.mockReturnValue({ textContent: "file1.ts" }); + mockQuerySelector.mockReturnValue({ textContent: "file1.ts", classList: { toggle: jest.fn() } }); (getSha1Hash as jest.Mock).mockResolvedValue("hash1"); // Mock the getDiffElementFileName method to return the correct file name @@ -585,7 +595,7 @@ describe("MessageToWebviewHandlerImpl", () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any await (handler as any).hideViewedFiles(mockDiffContainer, mockViewedState); - expect(mockToggle.click).toHaveBeenCalled(); + expect(mockToggle.checked).toBe(true); }); it("should mark files as changed since viewed", async () => { @@ -601,10 +611,14 @@ describe("MessageToWebviewHandlerImpl", () => { const mockDiffContainer = { querySelectorAll: jest.fn().mockReturnValue([mockToggle]), + style: {}, } as unknown as HTMLElement; mockClosest.mockReturnValue(mockFileContainer); - mockQuerySelector.mockReturnValue({ textContent: "file1.ts" }); + mockQuerySelector.mockReturnValue({ + textContent: "file1.ts", + classList: { toggle: jest.fn() }, + }); (getSha1Hash as jest.Mock).mockResolvedValue("different-hash"); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -639,7 +653,7 @@ describe("MessageToWebviewHandlerImpl", () => { (handler as any).getDiffHash = jest.fn().mockResolvedValue("test-hash"); // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (handler as any).sendFileViewedMessage(mockToggle as unknown as HTMLInputElement); + await (handler as any).sendFileViewedMessage(mockToggle as unknown as HTMLInputElement, true); expect(mockPostMessageToExtensionFn).toHaveBeenCalledWith({ kind: "toggleFileViewed", @@ -785,6 +799,7 @@ describe("MessageToWebviewHandlerImpl", () => { const mockDiffContainer = { querySelectorAll: mockQuerySelectorAll, addEventListener: mockAddEventListener, + style: {}, } as unknown as HTMLElement; const mockViewedToggles = [ @@ -810,8 +825,8 @@ describe("MessageToWebviewHandlerImpl", () => { await handler.updateWebview({ config: mockConfig, diffFiles: [createMockDiffFile("test.ts")], - diffContainer: "test-container", viewedState: mockViewedState, + collapseAll: false, }); expect(Diff2HtmlUI).toHaveBeenCalled(); From 1353029596c2d40d08d2bb925131d10247be4399 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:01:29 +0100 Subject: [PATCH 07/13] Fix CHANGED_SINCE_VIEWED behaviour on toggling all This omission was spotted by CoPilot --- src/webview/message/handler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index 3f7f67f..e359846 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -123,6 +123,7 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple for (const toggle of Array.from(allToggles)) { this.updateDiff2HtmlFileCollapsed(toggle, viewed); + toggle.classList.remove(CHANGED_SINCE_VIEWED); // no await here so we synchronously return from this function, the sending will happen later as hashes get computed this.sendFileViewedMessage(toggle, viewed); } From 48893b77a045055a37f5d19b6982e5b346539e76 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:01:54 +0100 Subject: [PATCH 08/13] :art: Implement two useful nit-picks from CoPilot --- src/extension/provider.ts | 2 +- src/webview/message/handler.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/provider.ts b/src/extension/provider.ts index beb8dd1..16f48f5 100644 --- a/src/extension/provider.ts +++ b/src/extension/provider.ts @@ -44,7 +44,7 @@ export class DiffViewerProvider implements vscode.CustomTextEditorProvider { vscode.commands.registerCommand("diffviewer.showLineByLine", () => setOutputFormatConfig("line-by-line")), vscode.commands.registerCommand("diffviewer.showSideBySide", () => setOutputFormatConfig("side-by-side")), vscode.commands.registerCommand("diffviewer.openCollapsed", async (file) => { - const collapsedUri = vscode.Uri.from({ ...file, query: "collapsed" }); + const collapsedUri = file.with({ query: "collapsed" }); await vscode.commands.executeCommand("vscode.openWith", collapsedUri, "diffViewer"); }), ]; diff --git a/src/webview/message/handler.ts b/src/webview/message/handler.ts index e359846..176d5f2 100644 --- a/src/webview/message/handler.ts +++ b/src/webview/message/handler.ts @@ -125,7 +125,7 @@ export class MessageToWebviewHandlerImpl extends GenericMessageHandlerImpl imple this.updateDiff2HtmlFileCollapsed(toggle, viewed); toggle.classList.remove(CHANGED_SINCE_VIEWED); // no await here so we synchronously return from this function, the sending will happen later as hashes get computed - this.sendFileViewedMessage(toggle, viewed); + void this.sendFileViewedMessage(toggle, viewed); } } From cd3d3605fca8abd4f5b870e54eb7b9c04a9c2a90 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:04:10 +0100 Subject: [PATCH 09/13] Fix test --- src/webview/message/__tests__/handler.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webview/message/__tests__/handler.test.ts b/src/webview/message/__tests__/handler.test.ts index f359f9b..d4d971e 100644 --- a/src/webview/message/__tests__/handler.test.ts +++ b/src/webview/message/__tests__/handler.test.ts @@ -381,8 +381,8 @@ describe("MessageToWebviewHandlerImpl", () => { const mockClosest = () => ({ querySelector: () => ({ classList: { toggle: jest.fn() } }) }); const mockToggles = [ - { checked: false, closest: mockClosest }, - { checked: false, closest: mockClosest }, + { checked: false, closest: mockClosest, classList: { remove: jest.fn() } }, + { checked: false, closest: mockClosest, classList: { remove: jest.fn() } }, ]; mockGetElementById.mockReturnValue(mockMarkAllCheckbox); From 4b386766131d7bd43416c99b3b38566f47f3baa7 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:51:45 +0100 Subject: [PATCH 10/13] Fix test running with vscode jest extension --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 504c6ed..854f1ce 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,7 @@ "pack:extension": "vsce package -o ./dist/vscode_diff_viewer_v$npm_package_version.vsix --githubBranch main", "run:webmode": "vscode-test-web --browserType=chromium --extensionDevelopmentPath=. --version=stable", "jest:run": "jest --silent", - "test": "npm run jest:run", + "test": "npm run jest:run --", "test:coverage": "jest --coverage --silent", "test:coverage:ci": "jest --coverage --ci --silent", "prepare": "husky install" From 8a4e0a10910c096c78cf41c7c2a6a528346df951 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:53:55 +0100 Subject: [PATCH 11/13] Clarify test The previous test would still pass if the two commands used the opposite setOutputFormatConfig calls. --- src/extension/__tests__/provider.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/extension/__tests__/provider.test.ts b/src/extension/__tests__/provider.test.ts index 19ca0cc..2831a3a 100644 --- a/src/extension/__tests__/provider.test.ts +++ b/src/extension/__tests__/provider.test.ts @@ -200,22 +200,22 @@ describe("DiffViewerProvider", () => { expect(mockRegisterCommand).toHaveBeenCalledWith("diffviewer.openCollapsed", expect.any(Function)); }); - it("should call setOutputFormatConfig when commands are executed", () => { + it.each([ + ["showSideBySide", "side-by-side"], + ["showLineByLine", "line-by-line"], + ])("should call setOutputFormatConfig when commands are executed", (cmd, expectedConfig) => { DiffViewerProvider.registerContributions({ extensionContext: mockExtensionContext, webviewPath: mockWebviewPath, }); - // Get the command functions - const lineByLineCommand = mockRegisterCommand.mock.calls[0][1]; - const sideBySideCommand = mockRegisterCommand.mock.calls[1][1]; + // Get the command function + const command = mockRegisterCommand.mock.calls.find((c) => c[0] == `diffviewer.${cmd}`)[1]; // Execute commands - lineByLineCommand(); - sideBySideCommand(); + command(); - expect(mockSetOutputFormatConfig).toHaveBeenCalledWith("line-by-line"); - expect(mockSetOutputFormatConfig).toHaveBeenCalledWith("side-by-side"); + expect(mockSetOutputFormatConfig).toHaveBeenCalledWith(expectedConfig); }); }); From ac5bb1f177b450404f409d53c41f00b0c0b63faa Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 22:54:12 +0100 Subject: [PATCH 12/13] Test the new command --- src/extension/__tests__/provider.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/extension/__tests__/provider.test.ts b/src/extension/__tests__/provider.test.ts index 2831a3a..5e72596 100644 --- a/src/extension/__tests__/provider.test.ts +++ b/src/extension/__tests__/provider.test.ts @@ -217,6 +217,23 @@ describe("DiffViewerProvider", () => { expect(mockSetOutputFormatConfig).toHaveBeenCalledWith(expectedConfig); }); + + it("should open a diff with all files collapsed", () => { + DiffViewerProvider.registerContributions({ + extensionContext: mockExtensionContext, + webviewPath: mockWebviewPath, + }); + + // Get the command function + const command = mockRegisterCommand.mock.calls.find((c) => c[0] == `diffviewer.openCollapsed`)[1]; + + (vscode.Uri.file as jest.Mock).mockReturnValueOnce({ with: jest.fn().mockReturnValue("uri-with-selected") }); + + // Execute commands + command(vscode.Uri.file("foo.patch")); + + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("vscode.openWith", "uri-with-selected", "diffViewer"); + }); }); describe("resolveCustomTextEditor", () => { From 6648e419eaa8d4fd598390c58f7eb05dc6c86968 Mon Sep 17 00:00:00 2001 From: Jacek Kopecky Date: Sat, 4 Oct 2025 23:02:52 +0100 Subject: [PATCH 13/13] Test collapseAll --- src/webview/message/__tests__/handler.test.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/webview/message/__tests__/handler.test.ts b/src/webview/message/__tests__/handler.test.ts index d4d971e..3aa076c 100644 --- a/src/webview/message/__tests__/handler.test.ts +++ b/src/webview/message/__tests__/handler.test.ts @@ -308,6 +308,9 @@ describe("MessageToWebviewHandlerImpl", () => { getPropertyValue: jest.fn().mockReturnValue("test-value"), }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const setAllViewedStates = ((handler as any).setAllViewedStates = jest.fn()); + await handler.updateWebview({ config: mockConfig, diffFiles: mockDiffFiles, @@ -316,6 +319,63 @@ describe("MessageToWebviewHandlerImpl", () => { }); expect(Diff2HtmlUI).toHaveBeenCalledWith(mockDiffContainer, mockDiffFiles, mockConfig.diff2html); + expect(setAllViewedStates).not.toHaveBeenCalled(); + }); + + it("should collapse all files if called with collapseAll", async () => { + const mockRoot = { + style: { setProperty: jest.fn() }, + }; + Object.defineProperty(global.document, "documentElement", { + value: mockRoot, + writable: true, + }); + + // Reset the mock to use the global implementation + mockGetElementById.mockReset(); + mockGetElementById.mockImplementation((id: string) => { + if (id === "diff-container") { + return mockDiffContainer; + } + if (id === "mark-all-viewed-checkbox") { + return { addEventListener: mockAddEventListener }; + } + if (id === "loading-container") { + return { style: { display: "none" } }; + } + if (id === "empty-message-container") { + return { style: { display: "none" } }; + } + if (id === "viewed-indicator") { + return { textContent: "" }; + } + if (id === "viewed-progress") { + return { style: { width: "" } }; + } + if (id === "mark-all-viewed-container") { + return { classList: { add: jest.fn(), remove: jest.fn() } }; + } + return { style: { display: "none" } }; + }); + + mockGetComputedStyle.mockReturnValue({ + getPropertyValue: jest.fn().mockReturnValue("test-value"), + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const setAllViewedStates = ((handler as any).setAllViewedStates = jest.fn()); + + await handler.updateWebview({ + config: mockConfig, + diffFiles: mockDiffFiles, + viewedState: mockViewedState, + collapseAll: true, + }); + + expect(Diff2HtmlUI).toHaveBeenCalledWith(mockDiffContainer, mockDiffFiles, mockConfig.diff2html); + + expect(setAllViewedStates).toHaveBeenCalledTimes(1); + expect(setAllViewedStates).toHaveBeenCalledWith(true); }); });