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/reviewFilesDiffView.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewFilesDiffView.ts index f778faf18..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,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, 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"; 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"; @@ -195,10 +196,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 +226,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 +302,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 +387,30 @@ 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 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. this.widget.setViewModel(viewModel, { preserveFocus: true, viewState, initialScrollPosition: this.document ? "top" : "firstChange" }); 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..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 @@ -207,19 +207,25 @@ 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 = 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", 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") { // 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;