From a4a26a247ebfacd52cf3c8d0ab0a5207e88516dd Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 8 May 2026 13:32:04 -0500 Subject: [PATCH] feat: add resizable panes --- .changeset/resize-panes.md | 5 +++ README.md | 1 + src/App.tsx | 48 ++++++++++++++++++++++++---- src/appCommands.ts | 45 +++++++++++++++++++++++++++ src/keymap/diffView.ts | 4 +++ src/keymap/listNav.ts | 16 ++++++++++ src/ui/PullRequestDiffPane.tsx | 57 ++++++++++++++++++++++++++++++++-- src/ui/diff.ts | 57 +++++++++++++++++++++++++--------- test/appCommands.test.ts | 33 ++++++++++++++++++++ test/diffStacking.test.ts | 21 +++++++++++++ 10 files changed, 263 insertions(+), 24 deletions(-) create mode 100644 .changeset/resize-panes.md diff --git a/.changeset/resize-panes.md b/.changeset/resize-panes.md new file mode 100644 index 0000000..d0a7c99 --- /dev/null +++ b/.changeset/resize-panes.md @@ -0,0 +1,5 @@ +--- +"@kitlangton/ghui": patch +--- + +Add split-pane resize shortcuts and expose them in the command palette. diff --git a/README.md b/README.md index dd27453..47475cd 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,7 @@ running. - `f`: open the changed-files navigator while viewing a diff - `left` / `right`: choose the deleted or added side while in split diff comment mode - `[` / `]`: switch files while viewing or commenting on a diff +- `<` / `>`: resize split panes in wide layout or split diff view - `s`: toggle draft or ready-for-review state - `m`: merge - `x`: close with confirmation diff --git a/src/App.tsx b/src/App.tsx index c4fdbad..3ad667c 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -263,6 +263,12 @@ const DETAIL_PREFETCH_BEHIND = 1 const DETAIL_PREFETCH_AHEAD = 3 const DETAIL_PREFETCH_CONCURRENCY = 3 const DETAIL_PREFETCH_DELAY_MS = 120 +const DEFAULT_SPLIT_RATIO = 0.56 +const DEFAULT_DIFF_SPLIT_RATIO = 0.5 +const SPLIT_RESIZE_COLUMNS = 4 +const MIN_LIST_PANE_WIDTH = 44 +const MIN_DETAIL_PANE_WIDTH = 28 +const MIN_DIFF_SIDE_WIDTH = 24 const appendPullRequestPage = (existing: readonly PullRequestItem[], incoming: readonly PullRequestItem[]) => { const seen = new Set(existing.map((pullRequest) => pullRequest.url)) const mergedIncoming = mergeCachedDetails(incoming, existing) @@ -342,6 +348,7 @@ const pullRequestsAtom = githubRuntime ) .pipe(Atom.keepAlive) const wrapIndex = (index: number, length: number) => (length === 0 ? 0 : ((index % length) + length) % length) +const clampNumber = (value: number, min: number, max: number) => Math.max(min, Math.min(value, max)) const selectedIndexAtom = Atom.make(0) const noticeAtom = Atom.make(null) const filterQueryAtom = Atom.make("") @@ -357,6 +364,8 @@ const diffScrollTopAtom = Atom.make(0) const diffRenderViewAtom = Atom.make("split") const diffWrapModeAtom = Atom.make("none") const diffWhitespaceModeAtom = Atom.make(initialDiffWhitespaceMode) +const diffSplitRatioAtom = Atom.make(DEFAULT_DIFF_SPLIT_RATIO) +const listDetailSplitRatioAtom = Atom.make(DEFAULT_SPLIT_RATIO) const diffCommentAnchorIndexAtom = Atom.make(0) const diffPreferredSideAtom = Atom.make(null) const diffCommentRangeStartIndexAtom = Atom.make(null) @@ -725,6 +734,8 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { const [diffRenderView, setDiffRenderView] = useAtom(diffRenderViewAtom) const [diffWrapMode, setDiffWrapMode] = useAtom(diffWrapModeAtom) const [diffWhitespaceMode, setDiffWhitespaceMode] = useAtom(diffWhitespaceModeAtom) + const [diffSplitRatio, setDiffSplitRatio] = useAtom(diffSplitRatioAtom) + const [listDetailSplitRatio, setListDetailSplitRatio] = useAtom(listDetailSplitRatioAtom) const [diffCommentAnchorIndex, setDiffCommentAnchorIndex] = useAtom(diffCommentAnchorIndexAtom) const [diffPreferredSide, setDiffPreferredSide] = useAtom(diffPreferredSideAtom) const [diffCommentRangeStartIndex, setDiffCommentRangeStartIndex] = useAtom(diffCommentRangeStartIndexAtom) @@ -839,8 +850,10 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { const isWideLayout = terminalWidth >= 100 const splitGap = 1 const sectionPadding = 1 - const leftPaneWidth = isWideLayout ? Math.max(44, Math.floor((contentWidth - splitGap) * 0.56)) : contentWidth - const rightPaneWidth = isWideLayout ? Math.max(28, contentWidth - leftPaneWidth - splitGap) : contentWidth + const splitAvailableWidth = Math.max(1, contentWidth - splitGap) + const maxListPaneWidth = Math.max(MIN_LIST_PANE_WIDTH, splitAvailableWidth - MIN_DETAIL_PANE_WIDTH) + const leftPaneWidth = isWideLayout ? clampNumber(Math.floor(splitAvailableWidth * listDetailSplitRatio), MIN_LIST_PANE_WIDTH, maxListPaneWidth) : contentWidth + const rightPaneWidth = isWideLayout ? Math.max(MIN_DETAIL_PANE_WIDTH, contentWidth - leftPaneWidth - splitGap) : contentWidth const dividerJunctionAt = Math.max(1, leftPaneWidth) const leftContentWidth = isWideLayout ? Math.max(24, leftPaneWidth - 2) : Math.max(24, contentWidth - sectionPadding * 2) const rightContentWidth = isWideLayout ? Math.max(24, rightPaneWidth - sectionPadding * 2) : Math.max(24, contentWidth - sectionPadding * 2) @@ -995,12 +1008,12 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { [selectedDiffState, readyDiffFiles], ) const stackedDiffFiles = useMemo( - () => buildStackedDiffFiles(readyDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth), - [readyDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth], + () => buildStackedDiffFiles(readyDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth, diffSplitRatio), + [readyDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth, diffSplitRatio], ) const diffCommentAnchors = useMemo( - () => (diffFullView ? getStackedDiffCommentAnchors(stackedDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth) : []), - [diffFullView, stackedDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth], + () => (diffFullView ? getStackedDiffCommentAnchors(stackedDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth, diffSplitRatio) : []), + [diffFullView, stackedDiffFiles, effectiveDiffRenderView, diffWrapMode, contentWidth, diffSplitRatio], ) const selectedDiffCommentAnchorIndex = Math.max(0, Math.min(diffCommentAnchorIndex, diffCommentAnchors.length - 1)) const selectedDiffCommentAnchor = diffCommentAnchors[selectedDiffCommentAnchorIndex] ?? null @@ -1607,6 +1620,21 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { const isSelectedPullRequestDetailLoading = selectedPullRequest !== null && !selectedPullRequest.detailLoaded const halfPage = Math.max(1, Math.floor(wideBodyHeight / 2)) + const resizeListPane = (columns: number) => { + if (!isWideLayout) return + const nextLeftPaneWidth = clampNumber(leftPaneWidth + columns, MIN_LIST_PANE_WIDTH, maxListPaneWidth) + setListDetailSplitRatio(nextLeftPaneWidth / splitAvailableWidth) + } + + const resizeDiffSplit = (columns: number) => { + if (effectiveDiffRenderView !== "split") return + const availableWidth = Math.max(1, contentWidth) + const minRatio = Math.min(0.5, MIN_DIFF_SIDE_WIDTH / availableWidth) + const maxRatio = 1 - minRatio + preserveCurrentDiffLocation() + setDiffSplitRatio((current) => clampNumber(current + columns / availableWidth, minRatio, maxRatio)) + } + const loadPullRequestReviewComments = (pullRequest: PullRequestItem, force = false) => { const key = pullRequestDiffKey(pullRequest) const previousLoadState = registry.get(diffCommentsLoadedAtom)[key] @@ -2929,6 +2957,7 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { loadedPullRequestCount, hasMorePullRequests, isLoadingMorePullRequests, + listSplitResizable: isWideLayout && !detailFullView && !diffFullView && !commentsViewActive, selectedPullRequest, detailFullView, diffFullView, @@ -2961,6 +2990,7 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { openRepositoryPicker, loadMorePullRequests, switchViewTo, + resizeListSplit: (delta) => resizeListPane(delta * SPLIT_RESIZE_COLUMNS), openDetails: () => { setDetailFullView(true) setDetailScrollOffset(0) @@ -2986,6 +3016,7 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { flashNotice(`Refreshing diff for #${selectedPullRequest.number}`) }, toggleDiffRenderView, + resizeDiffSplit: (delta) => resizeDiffSplit(delta * SPLIT_RESIZE_COLUMNS), toggleDiffWrapMode, toggleDiffWhitespaceMode, openChangedFilesModal, @@ -3306,6 +3337,8 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { toggleRange: () => runCommandById("diff.toggle-range"), toggleView: () => runCommandById("diff.toggle-view"), toggleWrap: () => runCommandById("diff.toggle-wrap"), + splitView: effectiveDiffRenderView === "split", + resizeSplit: (delta) => resizeDiffSplit(delta * SPLIT_RESIZE_COLUMNS), reload: () => runCommandById("diff.reload"), nextThread: () => runCommandById("diff.next-thread"), previousThread: () => runCommandById("diff.previous-thread"), @@ -3355,12 +3388,14 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { visibleCount: visiblePullRequests.length, hasFilter: filterQuery.length > 0, canScrollDetailPreview: isWideLayout && selectedPullRequest !== null, + canResizeSplit: isWideLayout && !detailFullView && !diffFullView && !commentsViewActive, runCommandById: (id) => { runCommandById(id) }, switchQueueMode, scrollDetailPreviewBy, scrollDetailPreviewTo, + resizeSplit: (delta) => resizeListPane(delta * SPLIT_RESIZE_COLUMNS), clearFilter: () => { runCommandById("filter.clear") }, @@ -3623,6 +3658,7 @@ export const App = ({ systemThemeGeneration = 0 }: AppProps) => { view={effectiveDiffRenderView} whitespaceMode={diffWhitespaceMode} wrapMode={diffWrapMode} + splitRatio={diffSplitRatio} paneWidth={contentWidth} height={wideBodyHeight} loadingIndicator={loadingIndicator} diff --git a/src/appCommands.ts b/src/appCommands.ts index abb6557..ac06e90 100644 --- a/src/appCommands.ts +++ b/src/appCommands.ts @@ -13,6 +13,7 @@ interface AppCommandActions { readonly openRepositoryPicker: () => void readonly loadMorePullRequests: () => void readonly switchViewTo: (view: PullRequestView) => void + readonly resizeListSplit: (delta: 1 | -1) => void readonly openDetails: () => void readonly closeDetails: () => void readonly openDiffView: () => void @@ -25,6 +26,7 @@ interface AppCommandActions { readonly openDeleteSelectedComment: () => void readonly reloadDiff: () => void readonly toggleDiffRenderView: () => void + readonly resizeDiffSplit: (delta: 1 | -1) => void readonly toggleDiffWrapMode: () => void readonly toggleDiffWhitespaceMode: () => void readonly openChangedFilesModal: () => void @@ -53,6 +55,7 @@ interface BuildAppCommandsInput { readonly loadedPullRequestCount: number readonly hasMorePullRequests: boolean readonly isLoadingMorePullRequests: boolean + readonly listSplitResizable: boolean readonly selectedPullRequest: PullRequestItem | null readonly detailFullView: boolean readonly diffFullView: boolean @@ -82,6 +85,7 @@ export const buildAppCommands = ({ loadedPullRequestCount, hasMorePullRequests, isLoadingMorePullRequests, + listSplitResizable, selectedPullRequest, detailFullView, diffFullView, @@ -108,6 +112,7 @@ export const buildAppCommands = ({ const selectedDiffLineReason = diffFullView && diffReady ? (selectedDiffCommentAnchorLabel ? null : "No diff line selected.") : diffOpenReadyReason const diffThreadReason = diffFullView && diffReady ? (hasDiffCommentThreads ? null : "No diff comments loaded.") : diffOpenReadyReason const changedFilesReason = diffFullView && diffReady ? (readyDiffFileCount > 0 ? null : "No changed files loaded.") : diffOpenReadyReason + const diffSplitReason = diffFullView ? (effectiveDiffRenderView === "split" ? null : "Split diff view is not active.") : "Open a diff first." const selectedCommentReason = selectedPullRequest ? (commentsViewActive ? (hasSelectedComment ? null : "No comment selected.") : "Open comments first.") : noPullRequestReason const ownCommentReason = selectedCommentReason ?? (canEditSelectedComment ? null : "Only your own (synced) comments can be edited or deleted.") const loadMoreDisabledReason = isLoadingMorePullRequests ? "Already loading more pull requests." : hasMorePullRequests ? null : "No more pull requests loaded by this view." @@ -195,6 +200,26 @@ export const buildAppCommands = ({ keywords: ["next page", "pagination", "more"], run: actions.loadMorePullRequests, }), + defineCommand({ + id: "list.resize-left", + title: "Resize list split left", + scope: "Navigation", + subtitle: "Give more space to pull request details", + shortcut: "<", + disabledReason: listSplitResizable ? null : "Split layout is not active.", + keywords: ["pane", "split", "width", "shrink list"], + run: () => actions.resizeListSplit(-1), + }), + defineCommand({ + id: "list.resize-right", + title: "Resize list split right", + scope: "Navigation", + subtitle: "Give more space to the pull request list", + shortcut: ">", + disabledReason: listSplitResizable ? null : "Split layout is not active.", + keywords: ["pane", "split", "width", "grow list"], + run: () => actions.resizeListSplit(1), + }), forSelected({ id: "detail.open", title: "Open pull request details", @@ -293,6 +318,26 @@ export const buildAppCommands = ({ disabledReason: diffFullView ? null : "Open a diff first.", run: actions.toggleDiffRenderView, }), + defineCommand({ + id: "diff.resize-split-left", + title: "Resize diff split left", + scope: "Diff", + subtitle: "Give more space to the right side", + shortcut: "<", + disabledReason: diffSplitReason, + keywords: ["pane", "split", "width", "old side"], + run: () => actions.resizeDiffSplit(-1), + }), + defineCommand({ + id: "diff.resize-split-right", + title: "Resize diff split right", + scope: "Diff", + subtitle: "Give more space to the left side", + shortcut: ">", + disabledReason: diffSplitReason, + keywords: ["pane", "split", "width", "new side"], + run: () => actions.resizeDiffSplit(1), + }), defineCommand({ id: "diff.toggle-wrap", title: "Toggle diff word wrap", diff --git a/src/keymap/diffView.ts b/src/keymap/diffView.ts index bb6e82b..9e2cd3b 100644 --- a/src/keymap/diffView.ts +++ b/src/keymap/diffView.ts @@ -11,6 +11,8 @@ export interface DiffViewCtx { readonly toggleRange: () => void readonly toggleView: () => void readonly toggleWrap: () => void + readonly splitView: boolean + readonly resizeSplit: (delta: number) => void readonly reload: () => void readonly nextThread: () => void readonly previousThread: () => void @@ -33,6 +35,8 @@ export const diffViewKeymap = Diff( { id: "diff.toggle-range", title: "Toggle comment range", keys: ["v"], run: (s) => s.toggleRange() }, { id: "diff.toggle-view", title: "Toggle split/unified", keys: ["shift+v"], run: (s) => s.toggleView() }, { id: "diff.toggle-wrap", title: "Toggle wrap", keys: ["w"], run: (s) => s.toggleWrap() }, + { id: "diff.resize-left", title: "Resize split left", keys: ["<"], enabled: (s) => (s.splitView ? true : "Split diff view is not active."), run: (s) => s.resizeSplit(-1) }, + { id: "diff.resize-right", title: "Resize split right", keys: [">"], enabled: (s) => (s.splitView ? true : "Split diff view is not active."), run: (s) => s.resizeSplit(1) }, { id: "diff.reload", title: "Reload diff", keys: ["r"], run: (s) => s.reload() }, { id: "diff.next-thread", title: "Next thread", keys: ["n"], run: (s) => s.nextThread() }, { id: "diff.previous-thread", title: "Previous thread", keys: ["p"], run: (s) => s.previousThread() }, diff --git a/src/keymap/listNav.ts b/src/keymap/listNav.ts index 9307fb0..8a9bffa 100644 --- a/src/keymap/listNav.ts +++ b/src/keymap/listNav.ts @@ -6,10 +6,12 @@ export interface ListNavCtx { readonly visibleCount: number readonly hasFilter: boolean readonly canScrollDetailPreview: boolean + readonly canResizeSplit: boolean readonly runCommandById: (id: string) => void readonly switchQueueMode: (delta: 1 | -1) => void readonly scrollDetailPreviewBy: (delta: number) => void readonly scrollDetailPreviewTo: (line: number) => void + readonly resizeSplit: (delta: number) => void readonly clearFilter: () => void readonly stepSelected: (delta: number) => void readonly stepSelectedUp: (count?: number) => void @@ -38,6 +40,20 @@ export const listNavKeymap = List( { id: "list.toggle-draft", title: "Toggle draft", keys: ["s", "shift+s"], run: (s) => s.runCommandById("pull.toggle-draft") }, { id: "list.copy", title: "Copy metadata", keys: ["y"], run: (s) => s.runCommandById("pull.copy-metadata") }, { id: "list.detail.open", title: "Open details", keys: ["return"], run: (s) => s.runCommandById("detail.open") }, + { + id: "list.resize-left", + title: "Resize split left", + keys: ["<"], + enabled: (s) => (s.canResizeSplit ? true : "Split layout is not active."), + run: (s) => s.resizeSplit(-1), + }, + { + id: "list.resize-right", + title: "Resize split right", + keys: [">"], + enabled: (s) => (s.canResizeSplit ? true : "Split layout is not active."), + run: (s) => s.resizeSplit(1), + }, // Queue mode tabs { id: "list.next-tab", title: "Next view", keys: ["tab"], run: (s) => s.switchQueueMode(1) }, diff --git a/src/ui/PullRequestDiffPane.tsx b/src/ui/PullRequestDiffPane.tsx index 94e4c99..74b5208 100644 --- a/src/ui/PullRequestDiffPane.tsx +++ b/src/ui/PullRequestDiffPane.tsx @@ -1,5 +1,5 @@ -import type { DiffRenderable, MouseEvent, ScrollBoxRenderable } from "@opentui/core" -import { useMemo, type Ref } from "react" +import type { DiffRenderable, MouseEvent, Renderable, ScrollBoxRenderable } from "@opentui/core" +import { useEffect, useMemo, useRef, type Ref } from "react" import type { DiffCommentSide, PullRequestItem, PullRequestReviewComment } from "../domain.js" import { colors, lineNumberTextColor, type ThemeId } from "./colors.js" import { CommentBodyLine, commentCountText, commentMetaSegments, CommentSegmentsLine } from "./comments.js" @@ -51,6 +51,36 @@ const FileStats = ({ stats }: { stats: DiffFileStats }) => { ) } +type SplitDiffInternals = { + readonly leftSide?: Renderable + readonly rightSide?: Renderable +} + +const percentWidth = (ratio: number) => `${(ratio * 100).toFixed(3)}%` as `${number}%` +let hasWarnedMissingSplitInternals = false + +const splitDiffInternals = (diff: DiffRenderable | null) => { + const candidate = diff as Partial | null + if (candidate?.leftSide && candidate.rightSide) return candidate as Required + + if (!hasWarnedMissingSplitInternals && process.env.NODE_ENV !== "production") { + hasWarnedMissingSplitInternals = true + console.warn("Unable to resize split diff: @opentui/core DiffRenderable no longer exposes leftSide/rightSide internals.") + } + return null +} + +// Called on attach, ratio changes, and diff render so the private split panes stay in sync with React state. +const applyDiffSplitRatio = (diff: DiffRenderable | null, splitRatio: number, requestRender = true) => { + const splitDiff = splitDiffInternals(diff) + if (!splitDiff) return false + + splitDiff.leftSide.width = percentWidth(splitRatio) + splitDiff.rightSide.width = percentWidth(1 - splitRatio) + if (requestRender) diff?.requestRender() + return true +} + const FileHeader = ({ file, index, @@ -89,6 +119,7 @@ export const PullRequestDiffPane = ({ view, whitespaceMode, wrapMode, + splitRatio, paneWidth, height, loadingIndicator, @@ -108,6 +139,7 @@ export const PullRequestDiffPane = ({ view: DiffView whitespaceMode: DiffWhitespaceMode wrapMode: DiffWrapMode + splitRatio: number paneWidth: number height: number loadingIndicator: string @@ -121,8 +153,16 @@ export const PullRequestDiffPane = ({ themeGeneration: number }) => { const readyFiles = diffState?._tag === "Ready" ? diffState.files : [] + const diffRefs = useRef(new Map()) const syntaxStyle = useMemo(() => createDiffSyntaxStyle(), [themeId, themeGeneration]) + useEffect(() => { + if (view !== "split") return + for (const diff of diffRefs.current.values()) { + applyDiffSplitRatio(diff, splitRatio) + } + }, [view, splitRatio, stackedFiles]) + if (!pullRequest) { return } @@ -215,9 +255,20 @@ export const PullRequestDiffPane = ({ setDiffRef(stackedFile.index, diff)} + ref={(diff: DiffRenderable | null) => { + if (diff) { + diffRefs.current.set(stackedFile.index, diff) + if (view === "split") applyDiffSplitRatio(diff, splitRatio) + } else { + diffRefs.current.delete(stackedFile.index) + } + setDiffRef(stackedFile.index, diff) + }} diff={stackedFile.file.patch} view={view} + renderBefore={function (this: DiffRenderable) { + if (view === "split") applyDiffSplitRatio(this, splitRatio, false) + }} syncScroll filetype={stackedFile.file.filetype ?? "text"} syntaxStyle={syntaxStyle} diff --git a/src/ui/diff.ts b/src/ui/diff.ts index 663ece4..29aac85 100644 --- a/src/ui/diff.ts +++ b/src/ui/diff.ts @@ -327,10 +327,16 @@ export const pullRequestDiffKey = (pullRequest: PullRequestItem) => `${pullReque export const safeDiffFileIndex = (files: readonly DiffFilePatch[], index: number) => (files.length > 0 ? Math.max(0, Math.min(index, files.length - 1)) : 0) -export const buildStackedDiffFiles = (files: readonly DiffFilePatch[], view: DiffView, wrapMode: DiffWrapMode, width: number): readonly StackedDiffFilePatch[] => { +export const buildStackedDiffFiles = ( + files: readonly DiffFilePatch[], + view: DiffView, + wrapMode: DiffWrapMode, + width: number, + splitRatio = 0.5, +): readonly StackedDiffFilePatch[] => { let offset = 0 return files.map((file, index) => { - const diffHeight = patchRenderableLineCount(file.patch, view, wrapMode, width) + const diffHeight = patchRenderableLineCount(file.patch, view, wrapMode, width, splitRatio) const separatorBefore = index === 0 ? 0 : 1 const headerLine = offset + separatorBefore const stackedFile = { @@ -380,9 +386,16 @@ export const diffCommentAnchorLabel = (anchor: Pick -const diffContentWidth = (lines: readonly string[], view: DiffView, width: number) => { +const diffContentWidths = (lines: readonly string[], view: DiffView, width: number, splitRatio = 0.5) => { const lineNumberGutterWidth = patchLineNumberGutterWidth(lines) - return view === "split" ? Math.max(1, Math.floor(width / 2) - lineNumberGutterWidth) : Math.max(1, width - lineNumberGutterWidth) + const splitLeftPaneWidth = Math.max(1, Math.floor(width * splitRatio) - lineNumberGutterWidth) + const splitRightPaneWidth = Math.max(1, width - Math.floor(width * splitRatio) - lineNumberGutterWidth) + const unifiedPaneWidth = Math.max(1, width - lineNumberGutterWidth) + return { + left: view === "split" ? splitLeftPaneWidth : unifiedPaneWidth, + right: view === "split" ? splitRightPaneWidth : unifiedPaneWidth, + unified: unifiedPaneWidth, + } } export const diffFileStats = (file: DiffFilePatch): DiffFileStats => { @@ -410,10 +423,16 @@ export const diffFileStatsText = (stats: DiffFileStats) => { return [stats.additions > 0 ? `+${stats.additions}` : null, stats.deletions > 0 ? `-${stats.deletions}` : null].filter((part): part is string => part !== null).join(" ") } -export const getDiffCommentAnchors = (file: DiffFilePatch, view: DiffView = "unified", wrapMode: DiffWrapMode = "none", width = 120): readonly DiffCommentAnchor[] => { +export const getDiffCommentAnchors = ( + file: DiffFilePatch, + view: DiffView = "unified", + wrapMode: DiffWrapMode = "none", + width = 120, + splitRatio = 0.5, +): readonly DiffCommentAnchor[] => { const anchors: DiffCommentAnchor[] = [] const lines = file.patch.split("\n") - const contentWidth = diffContentWidth(lines, view, width) + const contentWidths = diffContentWidths(lines, view, width, splitRatio) let oldLine = 0 let newLine = 0 let renderLine = 0 @@ -503,14 +522,14 @@ export const getDiffCommentAnchors = (file: DiffFilePatch, view: DiffView = "uni if (firstChar === "+") { const text = line.slice(1) - additions.push({ path: file.name, line: newLine, side: "RIGHT", kind: "addition", text, height: estimatedWrappedLineCount(text, contentWidth, wrapMode) }) + additions.push({ path: file.name, line: newLine, side: "RIGHT", kind: "addition", text, height: estimatedWrappedLineCount(text, contentWidths.right, wrapMode) }) newLine++ continue } if (firstChar === "-") { const text = line.slice(1) - deletions.push({ path: file.name, line: oldLine, side: "LEFT", kind: "deletion", text, height: estimatedWrappedLineCount(text, contentWidth, wrapMode) }) + deletions.push({ path: file.name, line: oldLine, side: "LEFT", kind: "deletion", text, height: estimatedWrappedLineCount(text, contentWidths.left, wrapMode) }) oldLine++ continue } @@ -518,7 +537,11 @@ export const getDiffCommentAnchors = (file: DiffFilePatch, view: DiffView = "uni if (firstChar === " ") { flushChangeBlock() const text = line.slice(1) - const anchor = { path: file.name, line: newLine, side: "RIGHT", kind: "context", text, height: estimatedWrappedLineCount(text, contentWidth, wrapMode) } as const + const height = + view === "split" + ? Math.max(estimatedWrappedLineCount(text, contentWidths.left, wrapMode), estimatedWrappedLineCount(text, contentWidths.right, wrapMode)) + : estimatedWrappedLineCount(text, contentWidths.unified, wrapMode) + const anchor = { path: file.name, line: newLine, side: "RIGHT", kind: "context", text, height } as const if (view === "split") { pushSplitContext(anchor) } else { @@ -540,9 +563,10 @@ export const getStackedDiffCommentAnchors = ( view: DiffView = "unified", wrapMode: DiffWrapMode = "none", width = 120, + splitRatio = 0.5, ): readonly StackedDiffCommentAnchor[] => stackedFiles.flatMap((stackedFile) => - getDiffCommentAnchors(stackedFile.file, view, wrapMode, width).map((anchor) => ({ + getDiffCommentAnchors(stackedFile.file, view, wrapMode, width, splitRatio).map((anchor) => ({ ...anchor, fileIndex: stackedFile.index, localRenderLine: anchor.renderLine, @@ -646,9 +670,9 @@ const patchLineNumberGutterWidth = (lines: readonly string[]) => { return Math.max(3, digits + 2) + (hasSigns ? 2 : 0) } -export const patchRenderableLineCount = (patch: string, view: DiffView, wrapMode: DiffWrapMode, width: number) => { +export const patchRenderableLineCount = (patch: string, view: DiffView, wrapMode: DiffWrapMode, width: number, splitRatio = 0.5) => { const lines = patch.split("\n") - const contentWidth = diffContentWidth(lines, view, width) + const contentWidths = diffContentWidths(lines, view, width, splitRatio) let count = 0 let inHunk = false let deletions: number[] = [] @@ -684,18 +708,21 @@ export const patchRenderableLineCount = (patch: string, view: DiffView, wrapMode if (firstChar === "\\") continue if (firstChar === "-") { - deletions.push(estimatedWrappedLineCount(line.slice(1), contentWidth, wrapMode)) + deletions.push(estimatedWrappedLineCount(line.slice(1), contentWidths.left, wrapMode)) continue } if (firstChar === "+") { - additions.push(estimatedWrappedLineCount(line.slice(1), contentWidth, wrapMode)) + additions.push(estimatedWrappedLineCount(line.slice(1), contentWidths.right, wrapMode)) continue } if (firstChar === " ") { flushChangeBlock() - count += estimatedWrappedLineCount(line.slice(1), contentWidth, wrapMode) + count += + view === "split" + ? Math.max(estimatedWrappedLineCount(line.slice(1), contentWidths.left, wrapMode), estimatedWrappedLineCount(line.slice(1), contentWidths.right, wrapMode)) + : estimatedWrappedLineCount(line.slice(1), contentWidths.unified, wrapMode) } } diff --git a/test/appCommands.test.ts b/test/appCommands.test.ts index cbd5afc..5d6f29b 100644 --- a/test/appCommands.test.ts +++ b/test/appCommands.test.ts @@ -39,6 +39,7 @@ const buildCommands = (overrides: Partial[0] loadedPullRequestCount: 1, hasMorePullRequests: false, isLoadingMorePullRequests: false, + listSplitResizable: true, selectedPullRequest, detailFullView: false, diffFullView: true, @@ -64,6 +65,7 @@ const buildCommands = (overrides: Partial[0] openRepositoryPicker: noop, loadMorePullRequests: noop, switchViewTo: noop, + resizeListSplit: noop, openDetails: noop, closeDetails: noop, openDiffView: noop, @@ -76,6 +78,7 @@ const buildCommands = (overrides: Partial[0] openDeleteSelectedComment: noop, reloadDiff: noop, toggleDiffRenderView: noop, + resizeDiffSplit: noop, toggleDiffWrapMode: noop, toggleDiffWhitespaceMode: noop, openChangedFilesModal: noop, @@ -103,6 +106,36 @@ const commandById = (id: string, overrides?: Partial { + test("list split resize commands are available from the command palette", () => { + const left = commandById("list.resize-left") + const right = commandById("list.resize-right") + + expect(left.shortcut).toBe("<") + expect(right.shortcut).toBe(">") + expect(left.disabledReason).toBeFalsy() + expect(right.disabledReason).toBeFalsy() + }) + + test("list split resize commands require split layout", () => { + expect(commandById("list.resize-left", { listSplitResizable: false }).disabledReason).toBe("Split layout is not active.") + expect(commandById("list.resize-right", { listSplitResizable: false }).disabledReason).toBe("Split layout is not active.") + }) + + test("diff split resize commands are available from a split diff", () => { + const left = commandById("diff.resize-split-left") + const right = commandById("diff.resize-split-right") + + expect(left.shortcut).toBe("<") + expect(right.shortcut).toBe(">") + expect(left.disabledReason).toBeFalsy() + expect(right.disabledReason).toBeFalsy() + }) + + test("diff split resize commands require split diff view", () => { + expect(commandById("diff.resize-split-left", { effectiveDiffRenderView: "unified" }).disabledReason).toBe("Split diff view is not active.") + expect(commandById("diff.resize-split-right", { diffFullView: false }).disabledReason).toBe("Open a diff first.") + }) + test("changed-files navigator is available from a ready diff", () => { const command = commandById("diff.changed-files") diff --git a/test/diffStacking.test.ts b/test/diffStacking.test.ts index 82494ff..3948a46 100644 --- a/test/diffStacking.test.ts +++ b/test/diffStacking.test.ts @@ -81,6 +81,27 @@ describe("stacked diff helpers", () => { expect(context).toMatchObject({ side: "RIGHT", renderLine: 4, colorLine: 3 }) }) + test("uses asymmetric split widths for wrapped split diffs", () => { + const [file] = splitPatchFiles(`diff --git a/ratio.ts b/ratio.ts +--- a/ratio.ts ++++ b/ratio.ts +@@ -1,2 +1,2 @@ +-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ++b + c`) + const anchors = getDiffCommentAnchors(file!, "split", "word", 60, 0.25) + const stacked = buildStackedDiffFiles([file!], "split", "word", 60, 0.25) + const deletion = anchors.find((anchor) => anchor.kind === "deletion")! + const addition = anchors.find((anchor) => anchor.kind === "addition")! + const context = anchors.find((anchor) => anchor.kind === "context")! + + expect(patchRenderableLineCount(file!.patch, "split", "word", 60, 0.25)).toBe(5) + expect(stacked[0]).toMatchObject({ diffHeight: 5 }) + expect(deletion).toMatchObject({ side: "LEFT", renderLine: 0, colorLine: 0 }) + expect(addition).toMatchObject({ side: "RIGHT", renderLine: 0, colorLine: 0 }) + expect(context).toMatchObject({ side: "RIGHT", renderLine: 4, colorLine: 4 }) + }) + test("chooses the first anchor at or below the current scroll line", () => { const stacked = buildStackedDiffFiles(splitPatchFiles(patch), "unified", "none", 120) const anchors = getStackedDiffCommentAnchors(stacked, "unified")