From de8205c6f56d08dbdf5c14dd9bbe50cb13502b98 Mon Sep 17 00:00:00 2001 From: sxl Date: Sat, 10 Oct 2026 13:18:07 +0100 Subject: [PATCH 1/4] Collapse identical comparisons into a single pane Automatically collapse document embeds to one pane when base and head are identical. Changed documents retain the selected Split/Unified layout. --- .../services/reviewDocumentDiffLayout.test.ts | 93 +++++++++++++++++++ .../services/reviewDocumentDiffLayout.ts | 37 ++++++++ .../vs/review/services/reviewFilesDiffView.ts | 27 ++++-- 3 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts create mode 100644 apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts new file mode 100644 index 000000000..088afb447 --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { Emitter } from "../../base/common/event.js"; +import { observableValue, transaction } from "../../base/common/observable.js"; +import type { ReviewDiffLayout } from "../common/reviewProtocol.js"; +import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; +import { + bindDocumentDiffLayout, + type DocumentDiffLayoutItem, +} from "./reviewDocumentDiffLayout.js"; + +test("identical documents collapse and changed or pending documents follow the preference", () => { + const changes = new Emitter(); + let preference: ReviewDiffLayout = "split"; + const layout = { + get: () => preference, + onDidChange: changes.event, + } as ReviewDiffLayoutSetting; + const isDiffUpToDate = observableValue("ready", false); + const diff = observableValue< + { identical: boolean; quitEarly: boolean } | undefined + >("diff", undefined); + const item = { isDiffUpToDate, diff }; + const items = observableValue< + readonly (DocumentDiffLayoutItem | undefined)[] + >("items", []); + let split: boolean | undefined; + const binding = bindDocumentDiffLayout(layout, items, (value) => { + split = value; + }); + try { + assert.equal( + split, + true, + "an empty/loading view must not count as identical", + ); + items.set([item], undefined); + assert.equal(split, true); + transaction((tx) => { + diff.set({ identical: true, quitEarly: false }, tx); + isDiffUpToDate.set(true, tx); + }); + assert.equal(split, false); + preference = "unified"; + changes.fire(preference); + preference = "split"; + changes.fire(preference); + assert.equal( + split, + false, + "choosing split must not duplicate identical documents", + ); + isDiffUpToDate.set(false, undefined); + assert.equal( + split, + true, + "a stale identical result must not collapse a changed document", + ); + transaction((tx) => { + diff.set({ identical: false, quitEarly: false }, tx); + isDiffUpToDate.set(true, tx); + }); + assert.equal(split, true); + preference = "unified"; + changes.fire(preference); + assert.equal( + split, + false, + "changed documents still respect the unified preference", + ); + preference = "split"; + changes.fire(preference); + diff.set({ identical: true, quitEarly: true }, undefined); + assert.equal( + split, + true, + "an incomplete comparison must not count as identical", + ); + diff.set({ identical: true, quitEarly: false }, undefined); + items.set([item, undefined], undefined); + assert.equal( + split, + true, + "missing sides and non-text entries must not collapse", + ); + items.set([item], undefined); + assert.equal(split, false); + } finally { + binding.dispose(); + changes.dispose(); + } +}); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts new file mode 100644 index 000000000..7ca0d3c98 --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts @@ -0,0 +1,37 @@ +import type { IDisposable } from "../../base/common/lifecycle.js"; +import { + autorun, + observableFromEvent, + type IObservable, +} from "../../base/common/observable.js"; +import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; + +export interface DocumentDiffLayoutItem { + readonly isDiffUpToDate: IObservable; + readonly diff: IObservable< + { identical: boolean; quitEarly: boolean } | undefined + >; +} + +/** Only completed, identical document comparisons override the user's layout. */ +export function bindDocumentDiffLayout( + layout: ReviewDiffLayoutSetting, + items: IObservable, + setRenderSideBySide: (split: boolean) => void, +): IDisposable { + const preference = observableFromEvent(layout.onDidChange, () => + layout.get(), + ); + return autorun((reader) => { + const current = items.read(reader); + const identical = + current.length > 0 && + current.every((item) => { + if (!item) return false; + const upToDate = item.isDiffUpToDate.read(reader); + const diff = item.diff.read(reader); + return upToDate && diff?.identical === true && !diff.quitEarly; + }); + setRenderSideBySide(preference.read(reader) === "split" && !identical); + }); +} diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index f778faf18..e6ebee853 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts @@ -11,11 +11,12 @@ import { Orientation, SplitView } from "../../base/browser/ui/splitview/splitvie import { disposableTimeout } from "../../base/common/async.js"; import { Emitter, Event } from "../../base/common/event.js"; import { Disposable, DisposableStore, MutableDisposable, toDisposable } from "../../base/common/lifecycle.js"; -import { autorun, observableValue } from "../../base/common/observable.js"; +import { autorun, derived, observableValue } from "../../base/common/observable.js"; import { isEqual } from "../../base/common/resources.js"; import { URI } from "../../base/common/uri.js"; import { ElementSizeObserver } from "../../editor/browser/config/elementSizeObserver.js"; import { isDiffEditor, type IDiffEditor } from "../../editor/browser/editorBrowser.js"; +import { DiffEditorViewModel } from "../../editor/browser/widget/diffEditor/diffEditorViewModel.js"; import { MultiDiffEditorViewModel } from "../../editor/browser/widget/multiDiffEditor/multiDiffEditorViewModel.js"; import { MultiDiffEditorWidget, type RevealOptions } from "../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidget.js"; import type { IMultiDiffEditorViewState } from "../../editor/browser/widget/multiDiffEditor/multiDiffEditorWidgetImpl.js"; @@ -37,6 +38,7 @@ import { REVIEW_COUNTS_PENDING_TOOLTIP, reviewChangesTooltip, reviewCountsToolti import { type ReviewDiffFileWire } from "../common/reviewProtocol.js"; import { binarySizeLabel, isBinaryCounts, type ReviewFileCounts } from "../common/reviewStructuralDiff.js"; import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; +import { bindDocumentDiffLayout } from "./reviewDocumentDiffLayout.js"; import { reviewMultiDiffLabelUris, ReviewMultiDiffUIElementFactory } from "./reviewMultiDiff.js"; import type { StructuralDiffSession } from "./reviewStructuralDiffSession.js"; @@ -195,10 +197,10 @@ export class ReviewFilesDiffView extends Disposable { private pendingViewState: IMultiDiffEditorViewState | undefined; private viewModel: MultiDiffEditorViewModel | undefined; private input: ReviewFilesEditorInput | undefined; - private readonly applyLayout: () => void; private readonly readyFiles = new Set(); private readonly fileStates = new Map(); private readonly settleHold = this._register(new MutableDisposable()); + private readonly documentLayout = this._register(new MutableDisposable()); /** Full structural counts for views without persisted coverage. */ private readonly streamStats = new Map< string, @@ -225,7 +227,7 @@ export class ReviewFilesDiffView extends Disposable { constructor( private readonly container: HTMLElement, overflowWidgetsDomNode: HTMLElement | undefined, - layout: ReviewDiffLayoutSetting, + private readonly diffLayout: ReviewDiffLayoutSetting, private readonly fileTreeContainer: HTMLElement | undefined, private readonly onToggleViewed: ((path: string, sectionId?: string) => void) | undefined, private readonly onToggleSection: ((id: string) => void) | undefined, @@ -301,11 +303,9 @@ export class ReviewFilesDiffView extends Disposable { ); // The widget's own switch, not the per-item option refresh: it pins the // width heuristic off, so the chosen layout is what renders at any width. - // A document snippet of an unchanged file reads as plain code. - this.applyLayout = () => this.widget.setRenderSideBySide(layout.get() === "split" - && !(this.document && this.input?.entries.every(entry => entry.file.status === "unchanged"))); - this._register(layout.onDidChange(this.applyLayout)); - this.applyLayout(); + const applyLayout = () => this.widget.setRenderSideBySide(this.diffLayout.get() === "split"); + if (!document) this._register(this.diffLayout.onDidChange(applyLayout)); + applyLayout(); if (document) { this._register(this.widget.onDidChangeContentHeight(() => { const height = Math.max(40, this.widget.getContentHeight()); @@ -388,13 +388,22 @@ export class ReviewFilesDiffView extends Disposable { async setInput(input: ReviewFilesEditorInput, viewState: IMultiDiffEditorViewState | undefined): Promise { this.settleHold.clear(); + this.documentLayout.clear(); this.pendingViewState = viewState; this.input = input; - this.applyLayout(); this.changedFilesTree?.setFiles(Array.from(new Map(input.entries.map(entry => [entry.file.path, entry.file])).values())); const viewModel = await input.getViewModel(); if (this._store.isDisposed) return; this.viewModel = viewModel; + if (this.document) { + const comparisons = derived(reader => viewModel.items.read(reader).map(item => { + // Missing sides and binaries are not identical text documents. + const entry = input.entries.find(entry => sameResource(item.originalUri, entry.original) && sameResource(item.modifiedUri, entry.modified)); + return item.originalUri && item.modifiedUri && entry && !entry.file.binary && entry.file.status !== "added" && entry.file.status !== "deleted" && item.diffEditorViewModel instanceof DiffEditorViewModel + ? item.diffEditorViewModel : undefined; + })); + this.documentLayout.value = bindDocumentDiffLayout(this.diffLayout, comparisons, split => this.widget.setRenderSideBySide(split)); + } // The canvas mounts this view without a user gesture, so the widget's // first-change navigation must never take keyboard focus. this.widget.setViewModel(viewModel, { preserveFocus: true, viewState, initialScrollPosition: this.document ? "top" : "firstChange" }); From 5137cb7683a93b7c27e64def83fb08f4b3593129 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sun, 11 Oct 2026 09:32:02 -0400 Subject: [PATCH 2/4] Show one pane for added and deleted files in peeks --- .../vs/review/services/reviewDocumentDiffLayout.test.ts | 4 +++- .../src/vs/review/services/reviewDocumentDiffLayout.ts | 9 +++++---- .../src/vs/review/services/reviewFilesDiffView.ts | 6 +++--- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts index 088afb447..560695f83 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts @@ -23,7 +23,7 @@ test("identical documents collapse and changed or pending documents follow the p >("diff", undefined); const item = { isDiffUpToDate, diff }; const items = observableValue< - readonly (DocumentDiffLayoutItem | undefined)[] + readonly (DocumentDiffLayoutItem | "one-sided" | undefined)[] >("items", []); let split: boolean | undefined; const binding = bindDocumentDiffLayout(layout, items, (value) => { @@ -86,6 +86,8 @@ test("identical documents collapse and changed or pending documents follow the p ); items.set([item], undefined); assert.equal(split, false); + items.set(["one-sided"], undefined); + assert.equal(split, false, "added and deleted files have one side to show"); } finally { binding.dispose(); changes.dispose(); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts index 7ca0d3c98..84280403f 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts @@ -13,10 +13,10 @@ export interface DocumentDiffLayoutItem { >; } -/** Only completed, identical document comparisons override the user's layout. */ +/** One-sided entries and completed, identical comparisons override the user's layout. */ export function bindDocumentDiffLayout( layout: ReviewDiffLayoutSetting, - items: IObservable, + items: IObservable, setRenderSideBySide: (split: boolean) => void, ): IDisposable { const preference = observableFromEvent(layout.onDidChange, () => @@ -24,14 +24,15 @@ export function bindDocumentDiffLayout( ); return autorun((reader) => { const current = items.read(reader); - const identical = + const onePane = current.length > 0 && current.every((item) => { if (!item) return false; + if (item === "one-sided") return true; const upToDate = item.isDiffUpToDate.read(reader); const diff = item.diff.read(reader); return upToDate && diff?.identical === true && !diff.quitEarly; }); - setRenderSideBySide(preference.read(reader) === "split" && !identical); + setRenderSideBySide(preference.read(reader) === "split" && !onePane); }); } diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index e6ebee853..3d2f693b0 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts @@ -397,10 +397,10 @@ export class ReviewFilesDiffView extends Disposable { this.viewModel = viewModel; if (this.document) { const comparisons = derived(reader => viewModel.items.read(reader).map(item => { - // Missing sides and binaries are not identical text documents. const entry = input.entries.find(entry => sameResource(item.originalUri, entry.original) && sameResource(item.modifiedUri, entry.modified)); - return item.originalUri && item.modifiedUri && entry && !entry.file.binary && entry.file.status !== "added" && entry.file.status !== "deleted" && item.diffEditorViewModel instanceof DiffEditorViewModel - ? item.diffEditorViewModel : undefined; + if (!entry || entry.file.binary) return undefined; + if (entry.file.status === "added" || entry.file.status === "deleted") return "one-sided" as const; + return item.diffEditorViewModel instanceof DiffEditorViewModel ? item.diffEditorViewModel : undefined; })); this.documentLayout.value = bindDocumentDiffLayout(this.diffLayout, comparisons, split => this.widget.setRenderSideBySide(split)); } From 16ef77b8ab03ce8b7973770c12344c92a64d52e4 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sun, 11 Oct 2026 09:32:02 -0400 Subject: [PATCH 3/4] Leave zero halves out of diff header and tree counts --- .../src/vs/review/browser/reviewChangedFilesTree.test.ts | 2 +- .../src/vs/review/browser/reviewChangedFilesTree.ts | 4 ++-- .../code-oss/src/vs/review/services/reviewMultiDiff.ts | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.test.ts b/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.test.ts index b6131a10d..986e930d2 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.test.ts @@ -55,7 +55,7 @@ test('a viewed file says "Viewed" in place of its counts', () => { test('a file with changes left to read shows its remaining counts, not greyed', () => { const unread = row('unread'); - assert.equal(unread.counts, '+3−0'); + assert.equal(unread.counts, '+3'); assert.ok(!unread.row.classList.contains('review-file-folded')); assert.ok(!unread.row.classList.contains('review-file-viewed')); }); diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.ts b/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.ts index ef09a6c88..23261dc0b 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/reviewChangedFilesTree.ts @@ -174,8 +174,8 @@ export class ChangedFilesTreeRenderer else if (progress?.state === 'viewed') template.counts.textContent = 'Viewed'; else if (progress?.state === 'folded') template.counts.textContent = 'Folded'; else if (additions !== undefined && deletions !== undefined) { - const added = append(template.counts, $('span.review-tree-added')); added.textContent = `+${compact(additions)}`; - const removed = append(template.counts, $('span.review-tree-removed')); removed.textContent = `−${compact(deletions)}`; + if (additions) append(template.counts, $('span.review-tree-added')).textContent = `+${compact(additions)}`; + if (deletions) append(template.counts, $('span.review-tree-removed')).textContent = `−${compact(deletions)}`; } const total = progress?.total ?? { additions: lines?.added ?? 0, deletions: lines?.removed ?? 0 }; if (!binary) template.countsTooltip.content = element.file.status === "unchanged" diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts index 1e4e00b6b..84cb3b0a2 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts @@ -207,13 +207,17 @@ export class ReviewMultiDiffUIElementFactory deletions.textContent = `−${compactCount(current.deletions!)}`; counts.setAttribute( "aria-label", - `${current.additions} lines added, ${current.deletions} lines removed`, + [current.additions ? `${current.additions} lines added` : "", current.deletions ? `${current.deletions} lines removed` : ""].filter(Boolean).join(", "), ); } countsTooltip.content = current.countsTooltip; + const settled = current.viewedState === "viewed" || current.viewedState === "folded"; + additions.hidden = current.additions === 0 && !settled; + deletions.hidden = current.deletions === 0; + counts.hidden ||= additions.hidden && deletions.hidden; const path = current.modified ?? current.original; viewed.update(current.viewedState, `${current.viewedScope === 'lens' ? 'selected lens in ' : ''}${path ? reviewMultiDiffLabelPath(path) : 'file'}`, !current.onToggleViewed); - counts.classList.toggle("review-counts-viewed", current.viewedState === "viewed" || current.viewedState === "folded"); + counts.classList.toggle("review-counts-viewed", settled); counts.classList.toggle("review-counts-folded", current.viewedState === "folded"); if (current.viewedState === "viewed") { additions.textContent = current.viewedScope === 'lens' ? 'Lens viewed' : 'Viewed'; deletions.textContent = ""; } else if (current.viewedState === "folded") { From 41f2ec21287118ae610cfc9a06058e3bbc5b10e3 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sun, 11 Oct 2026 11:21:29 -0400 Subject: [PATCH 4/4] Inline the peek layout binding and hide zero halves only for numbers --- .../services/reviewDocumentDiffLayout.test.ts | 95 ------------------- .../services/reviewDocumentDiffLayout.ts | 38 -------- .../vs/review/services/reviewFilesDiffView.ts | 25 +++-- .../src/vs/review/services/reviewMultiDiff.ts | 8 +- 4 files changed, 21 insertions(+), 145 deletions(-) delete mode 100644 apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts delete mode 100644 apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts deleted file mode 100644 index 560695f83..000000000 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.test.ts +++ /dev/null @@ -1,95 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { Emitter } from "../../base/common/event.js"; -import { observableValue, transaction } from "../../base/common/observable.js"; -import type { ReviewDiffLayout } from "../common/reviewProtocol.js"; -import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; -import { - bindDocumentDiffLayout, - type DocumentDiffLayoutItem, -} from "./reviewDocumentDiffLayout.js"; - -test("identical documents collapse and changed or pending documents follow the preference", () => { - const changes = new Emitter(); - let preference: ReviewDiffLayout = "split"; - const layout = { - get: () => preference, - onDidChange: changes.event, - } as ReviewDiffLayoutSetting; - const isDiffUpToDate = observableValue("ready", false); - const diff = observableValue< - { identical: boolean; quitEarly: boolean } | undefined - >("diff", undefined); - const item = { isDiffUpToDate, diff }; - const items = observableValue< - readonly (DocumentDiffLayoutItem | "one-sided" | undefined)[] - >("items", []); - let split: boolean | undefined; - const binding = bindDocumentDiffLayout(layout, items, (value) => { - split = value; - }); - try { - assert.equal( - split, - true, - "an empty/loading view must not count as identical", - ); - items.set([item], undefined); - assert.equal(split, true); - transaction((tx) => { - diff.set({ identical: true, quitEarly: false }, tx); - isDiffUpToDate.set(true, tx); - }); - assert.equal(split, false); - preference = "unified"; - changes.fire(preference); - preference = "split"; - changes.fire(preference); - assert.equal( - split, - false, - "choosing split must not duplicate identical documents", - ); - isDiffUpToDate.set(false, undefined); - assert.equal( - split, - true, - "a stale identical result must not collapse a changed document", - ); - transaction((tx) => { - diff.set({ identical: false, quitEarly: false }, tx); - isDiffUpToDate.set(true, tx); - }); - assert.equal(split, true); - preference = "unified"; - changes.fire(preference); - assert.equal( - split, - false, - "changed documents still respect the unified preference", - ); - preference = "split"; - changes.fire(preference); - diff.set({ identical: true, quitEarly: true }, undefined); - assert.equal( - split, - true, - "an incomplete comparison must not count as identical", - ); - diff.set({ identical: true, quitEarly: false }, undefined); - items.set([item, undefined], undefined); - assert.equal( - split, - true, - "missing sides and non-text entries must not collapse", - ); - items.set([item], undefined); - assert.equal(split, false); - items.set(["one-sided"], undefined); - assert.equal(split, false, "added and deleted files have one side to show"); - } finally { - binding.dispose(); - changes.dispose(); - } -}); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts deleted file mode 100644 index 84280403f..000000000 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDocumentDiffLayout.ts +++ /dev/null @@ -1,38 +0,0 @@ -import type { IDisposable } from "../../base/common/lifecycle.js"; -import { - autorun, - observableFromEvent, - type IObservable, -} from "../../base/common/observable.js"; -import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; - -export interface DocumentDiffLayoutItem { - readonly isDiffUpToDate: IObservable; - readonly diff: IObservable< - { identical: boolean; quitEarly: boolean } | undefined - >; -} - -/** One-sided entries and completed, identical comparisons override the user's layout. */ -export function bindDocumentDiffLayout( - layout: ReviewDiffLayoutSetting, - items: IObservable, - setRenderSideBySide: (split: boolean) => void, -): IDisposable { - const preference = observableFromEvent(layout.onDidChange, () => - layout.get(), - ); - return autorun((reader) => { - const current = items.read(reader); - const onePane = - current.length > 0 && - current.every((item) => { - if (!item) return false; - if (item === "one-sided") return true; - const upToDate = item.isDiffUpToDate.read(reader); - const diff = item.diff.read(reader); - return upToDate && diff?.identical === true && !diff.quitEarly; - }); - setRenderSideBySide(preference.read(reader) === "split" && !onePane); - }); -} diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index 3d2f693b0..3a1bfe262 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts @@ -11,7 +11,7 @@ import { Orientation, SplitView } from "../../base/browser/ui/splitview/splitvie import { disposableTimeout } from "../../base/common/async.js"; import { Emitter, Event } from "../../base/common/event.js"; import { Disposable, DisposableStore, MutableDisposable, toDisposable } from "../../base/common/lifecycle.js"; -import { autorun, derived, observableValue } from "../../base/common/observable.js"; +import { autorun, observableFromEvent, observableValue } from "../../base/common/observable.js"; import { isEqual } from "../../base/common/resources.js"; import { URI } from "../../base/common/uri.js"; import { ElementSizeObserver } from "../../editor/browser/config/elementSizeObserver.js"; @@ -38,7 +38,6 @@ import { REVIEW_COUNTS_PENDING_TOOLTIP, reviewChangesTooltip, reviewCountsToolti import { type ReviewDiffFileWire } from "../common/reviewProtocol.js"; import { binarySizeLabel, isBinaryCounts, type ReviewFileCounts } from "../common/reviewStructuralDiff.js"; import type { ReviewDiffLayoutSetting } from "./reviewDiffLayout.js"; -import { bindDocumentDiffLayout } from "./reviewDocumentDiffLayout.js"; import { reviewMultiDiffLabelUris, ReviewMultiDiffUIElementFactory } from "./reviewMultiDiff.js"; import type { StructuralDiffSession } from "./reviewStructuralDiffSession.js"; @@ -396,13 +395,21 @@ export class ReviewFilesDiffView extends Disposable { if (this._store.isDisposed) return; this.viewModel = viewModel; if (this.document) { - const comparisons = derived(reader => viewModel.items.read(reader).map(item => { - const entry = input.entries.find(entry => sameResource(item.originalUri, entry.original) && sameResource(item.modifiedUri, entry.modified)); - if (!entry || entry.file.binary) return undefined; - if (entry.file.status === "added" || entry.file.status === "deleted") return "one-sided" as const; - return item.diffEditorViewModel instanceof DiffEditorViewModel ? item.diffEditorViewModel : undefined; - })); - this.documentLayout.value = bindDocumentDiffLayout(this.diffLayout, comparisons, split => this.widget.setRenderSideBySide(split)); + const preference = observableFromEvent(this.diffLayout.onDidChange, () => this.diffLayout.get()); + // Unchanged and one-sided files, and completed identical comparisons, override the user's layout. + this.documentLayout.value = autorun(reader => { + const items = viewModel.items.read(reader); + const onePane = items.length > 0 && items.every(item => { + const entry = input.entries.find(entry => sameResource(item.originalUri, entry.original) && sameResource(item.modifiedUri, entry.modified)); + if (!entry || entry.file.binary) return false; + if (entry.file.status === "added" || entry.file.status === "deleted" || entry.file.status === "unchanged") return true; + const comparison = item.diffEditorViewModel; + if (!(comparison instanceof DiffEditorViewModel)) return false; + const diff = comparison.diff.read(reader); + return comparison.isDiffUpToDate.read(reader) && diff?.identical === true && !diff.quitEarly; + }); + this.widget.setRenderSideBySide(preference.read(reader) === "split" && !onePane); + }); } // The canvas mounts this view without a user gesture, so the widget's // first-change navigation must never take keyboard focus. diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts index 84cb3b0a2..4962d3716 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewMultiDiff.ts @@ -212,9 +212,7 @@ export class ReviewMultiDiffUIElementFactory } countsTooltip.content = current.countsTooltip; const settled = current.viewedState === "viewed" || current.viewedState === "folded"; - additions.hidden = current.additions === 0 && !settled; - deletions.hidden = current.deletions === 0; - counts.hidden ||= additions.hidden && deletions.hidden; + additions.hidden = deletions.hidden = false; const path = current.modified ?? current.original; viewed.update(current.viewedState, `${current.viewedScope === 'lens' ? 'selected lens in ' : ''}${path ? reviewMultiDiffLabelPath(path) : 'file'}`, !current.onToggleViewed); counts.classList.toggle("review-counts-viewed", settled); @@ -224,6 +222,10 @@ export class ReviewMultiDiffUIElementFactory // A hidden file's note already says why it is folded. additions.textContent = "Folded"; deletions.textContent = ""; counts.hidden ||= !!current.note; + } else { + additions.hidden = !current.additions; + deletions.hidden = !current.deletions; + counts.hidden ||= additions.hidden && deletions.hidden; } note.hidden = !current.note;