diff --git a/Sources/termio/Git/DiffDocument.swift b/Sources/termio/Git/DiffDocument.swift index cee17619..b06314e8 100644 --- a/Sources/termio/Git/DiffDocument.swift +++ b/Sources/termio/Git/DiffDocument.swift @@ -57,6 +57,8 @@ final class DiffDocument { static let bandFont = NSFont.systemFont(ofSize: 10.5, weight: .medium) /// Extra breathing room drawn around a band row (the fill is expanded to match). static let bandPadding: CGFloat = 3 + /// Leading added between every line — a touch of air so the diff doesn't read as a dense wall. + static let codeLineSpacing: CGFloat = 4 // MARK: Building @@ -65,8 +67,16 @@ final class DiffDocument { /// than a handful of lines collapse to a band keeping 3 lines of context on the /// side(s) that face a change, and `expanded` bands splice their lines back in. static func build(rows: [DiffRow], expanded: Set, codeFont: NSFont) -> DiffDocument { - let items = displayItems(rows: rows, expanded: expanded) + build(items: displayItems(rows: rows, expanded: expanded), allRows: rows, codeFont: codeFont) + } + /// Composes several files into one stacked document (github.com "Files changed"): each file's + /// folded rows, prefixed by a full-width header row. Row ids **must** already be globally unique + /// across files (offset per file upstream) so band expansion and the syntax pass stay + /// unambiguous. One document means one scroll, and selection / ⌘F run continuously across files. + /// The shared assembly: lays the display items down as one attributed string with per-paragraph + /// metadata. `allRows` sizes the gutter columns (which sides carry numbers, and the widest). + private static func build(items: [DisplayItem], allRows: [DiffRow], codeFont: NSFont) -> DiffDocument { var text = String() text.reserveCapacity(items.reduce(0) { $0 + $1.textLength + 1 }) var lines: [Line] = [] @@ -98,13 +108,19 @@ final class DiffDocument { .font: codeFont, .foregroundColor: NSColor.labelColor, ]) + // A little air between lines — the diff reads tighter than prose, so a few points of leading + // lift the whole document. Bands and headers re-set their own styles below, with the same lift. + let baseStyle = NSMutableParagraphStyle() + baseStyle.lineSpacing = codeLineSpacing + attributed.addAttribute(.paragraphStyle, value: baseStyle, + range: NSRange(location: 0, length: attributed.length)) styleBandsAndEmphasis(attributed, items: items, lines: lines) return DiffDocument( attributed: attributed, lines: lines, - hasOldGutter: rows.contains { $0.kind != .hunk && $0.oldLine != nil }, - hasNewGutter: rows.contains { $0.kind != .hunk && $0.newLine != nil }, + hasOldGutter: allRows.contains { $0.kind != .hunk && $0.oldLine != nil }, + hasNewGutter: allRows.contains { $0.kind != .hunk && $0.newLine != nil }, maxLineNumber: maxLineNumber ) } @@ -137,6 +153,7 @@ final class DiffDocument { items: [DisplayItem], lines: [Line]) { let bandStyle = NSMutableParagraphStyle() bandStyle.alignment = .center + bandStyle.lineSpacing = codeLineSpacing bandStyle.paragraphSpacingBefore = bandPadding bandStyle.paragraphSpacing = bandPadding diff --git a/Sources/termio/Git/DiffTextView.swift b/Sources/termio/Git/DiffTextView.swift index 36a5ecba..e96b6d70 100644 --- a/Sources/termio/Git/DiffTextView.swift +++ b/Sources/termio/Git/DiffTextView.swift @@ -35,6 +35,11 @@ struct DiffTextPane: NSViewRepresentable { /// Bumped when the find bar closes, so the text view reclaims first responder and its /// ← / → walk and Esc work again. var reclaimFocus: Int = 0 + /// Embedded mode: the pane is stacked inside an outer scroll (the multi-file card list), so it + /// must not scroll or grab focus itself — it grows to its content and reports that height back so + /// the SwiftUI card can size to it, letting the outer list own the scroll. + var embedded: Bool = false + var onContentHeight: ((CGFloat) -> Void)? = nil func makeCoordinator() -> Coordinator { Coordinator() } @@ -68,6 +73,13 @@ struct DiffTextPane: NSViewRepresentable { scrollView.documentView = textView scrollView.hasVerticalScroller = true scrollView.hasHorizontalScroller = false + // Embedded panes are sized to their content by the SwiftUI card, so they never actually scroll + // — but keep the scroll view otherwise standard (only auto-hide the overlay scroller) so its + // ruler still lays out to the full height. Disabling the scroller knocks the gutter short. + if embedded { + scrollView.autohidesScrollers = true + scrollView.scrollerStyle = .overlay + } scrollView.borderType = .noBorder scrollView.drawsBackground = true scrollView.backgroundColor = backgroundColor @@ -89,6 +101,10 @@ struct DiffTextPane: NSViewRepresentable { // exposed strip, so without a full invalidation the gutter's absolutely // positioned numbers desync into a garbled smear (the editor's ruler learned // this the hard way). + context.coordinator.scrollView = scrollView + context.coordinator.embedded = embedded + context.coordinator.onContentHeight = onContentHeight + textView.postsFrameChangedNotifications = true context.coordinator.observeFrame(of: textView) scrollView.contentView.postsBoundsChangedNotifications = true @@ -96,13 +112,16 @@ struct DiffTextPane: NSViewRepresentable { apply(to: textView, layoutManager: layoutManager, ruler: ruler, coordinator: context.coordinator) - - // Keys (← → walk, Esc) should work the moment the overlay lands, without - // a click first. Deferred one turn — at make time the view has no window yet. - DispatchQueue.main.async { [weak textView] in - guard let textView, let window = textView.window else { return } - if window.firstResponder === window || window.firstResponder is NSTextView == false { - window.makeFirstResponder(textView) + context.coordinator.reportHeightIfNeeded() + + // Keys (← → walk, Esc) should work the moment the overlay lands, without a click first — + // but an embedded pane must not steal focus (many stacked panes would fight over it). + if !embedded { + DispatchQueue.main.async { [weak textView] in + guard let textView, let window = textView.window else { return } + if window.firstResponder === window || window.firstResponder is NSTextView == false { + window.makeFirstResponder(textView) + } } } return scrollView @@ -127,6 +146,10 @@ struct DiffTextPane: NSViewRepresentable { context.coordinator.updateFind(query: findQuery, options: findOptions, focusedIndex: findFocusedIndex, in: textView) context.coordinator.reclaimFocusIfNeeded(reclaimFocus, in: textView) + + context.coordinator.onContentHeight = onContentHeight + // A band-expand or a syntax pass changes the content height; re-measure so the card resizes. + context.coordinator.reportHeightIfNeeded() } /// Swaps the document in when it changed (initial load, band expand) and lays the @@ -180,6 +203,27 @@ struct DiffTextPane: NSViewRepresentable { var appliedStyled: Int? weak var ruler: DiffGutterRulerView? var onMatchesChanged: ((Int) -> Void)? + // Embedded (card-stacked) sizing: measure the laid-out content and hand its height to SwiftUI. + weak var scrollView: NSScrollView? + var embedded = false + var onContentHeight: ((CGFloat) -> Void)? + private var reportedHeight: CGFloat = -1 + + /// Measure the text's laid-out height and report it (once, on change) so the SwiftUI card sizes + /// to fit and the outer list scrolls. No-op unless embedded. + func reportHeightIfNeeded() { + guard embedded, let scrollView, + let textView = scrollView.documentView as? DiffTextView, + let layoutManager = textView.layoutManager, + let container = textView.textContainer else { return } + layoutManager.ensureLayout(for: container) + let height = layoutManager.usedRect(for: container).height + + textView.textContainerInset.height * 2 + guard abs(height - reportedHeight) > 0.5 else { return } + reportedHeight = height + let callback = onContentHeight + DispatchQueue.main.async { callback?(height) } + } /// The same incremental-find engine the code editor uses — highlights and semantics /// stay identical across the two ⌘F surfaces. private let find = TextFindEngine() @@ -231,7 +275,11 @@ struct DiffTextPane: NSViewRepresentable { NotificationCenter.default.addObserver( forName: NSView.frameDidChangeNotification, object: textView, queue: .main ) { [weak self] _ in - MainActor.assumeIsolated { self?.ruler?.needsDisplay = true } + MainActor.assumeIsolated { + self?.ruler?.needsDisplay = true + // A width change re-wraps the text, so the content height moves — re-measure. + self?.reportHeightIfNeeded() + } } } diff --git a/Sources/termio/Git/PRFilesDiffView.swift b/Sources/termio/Git/PRFilesDiffView.swift index e29c0a02..c4a725dc 100644 --- a/Sources/termio/Git/PRFilesDiffView.swift +++ b/Sources/termio/Git/PRFilesDiffView.swift @@ -19,22 +19,24 @@ struct PRFilesSplitView: View { @Environment(\.colorScheme) private var colorScheme @State private var selection: String? - /// Narrow mode only: whether we've drilled from the file list into a file's diff. - @State private var narrowShowingDiff = false - /// The list rail's width — the changed-files column, a touch wider than the sidebar - /// since PR paths run deep. - private static let listWidth: CGFloat = 260 - /// Below this the file list and the diff can't both breathe, so the view collapses to a single - /// navigable column (list → tap → diff → back) — the same single-column, master→detail idiom the - /// issue/PR detail (and the whole inspector) already uses. Above it, side-by-side has room. - private static let sideBySideMinWidth: CGFloat = 620 + /// The changed-files rail width for a given pane width — a third of the pane, clamped so neither + /// the list (deep PR paths) nor the diff starves as the right section resizes. Fixed-width rails + /// force an all-or-nothing breakpoint; scaling it lets side-by-side engage far sooner. + private static func railWidth(for paneWidth: CGFloat) -> CGFloat { + min(280, max(190, paneWidth * 0.32)) + } + /// Below this the file list and a usable diff can't both fit, so the view collapses to a single + /// navigable column (list → tap → diff → back) — the same master→detail idiom the issue/PR detail + /// (and the whole inspector) already uses. Driven purely by the *right section's* own width, so + /// widening the inspector — or maximizing — brings the list back beside the diff. + private static let sideBySideMinWidth: CGFloat = 500 var body: some View { GeometryReader { geo in Group { if geo.size.width >= Self.sideBySideMinWidth { - sideBySide + sideBySide(railWidth: Self.railWidth(for: geo.size.width)) } else { narrow } @@ -51,10 +53,10 @@ struct PRFilesSplitView: View { // MARK: Layouts /// Wide: GitHub Desktop's side-by-side — the file list rail beside the selected file's diff. - private var sideBySide: some View { + private func sideBySide(railWidth: CGFloat) -> some View { HStack(spacing: 0) { fileList(onSelect: { _ in }) - .frame(width: Self.listWidth) + .frame(width: railWidth) .background(Color(nsColor: settings.terminalBackgroundColor).opacity(0.4)) Rectangle() .fill(Color.primary.opacity(0.08)) @@ -64,43 +66,14 @@ struct PRFilesSplitView: View { } } - /// Narrow: one column at a time — the file list, or (after a tap) the selected file's diff behind - /// a "‹ Files" back bar. The arrow-key walk inside the diff still steps files without popping out. - @ViewBuilder + /// Narrow: no room for a list rail beside a readable diff, so every file's diff stacks in one + /// continuous scroll (github.com "Files changed"), the files separated by full-width headers — + /// you see *all* the files, not one at a time, and selection / ⌘F run across the whole set. private var narrow: some View { - if narrowShowingDiff, selection != nil { - VStack(spacing: 0) { - narrowBackBar - detail.frame(maxWidth: .infinity, maxHeight: .infinity) - } - } else { - fileList(onSelect: { _ in narrowShowingDiff = true }) - .frame(maxWidth: .infinity, maxHeight: .infinity) - } - } - - /// The narrow-mode back control, sized like the pane's other header bars. - private var narrowBackBar: some View { - HStack(spacing: 4) { - Button { narrowShowingDiff = false } label: { - HStack(spacing: 2) { - Image(systemName: "chevron.left").font(.system(size: 11, weight: .semibold)) - Text("Files").font(.system(size: 12, weight: .medium)) - } - } - .buttonStyle(.plain) - .foregroundStyle(.secondary) - Spacer(minLength: 0) - Text("\(files.count) file\(files.count == 1 ? "" : "s")") - .font(.system(size: 10.5, weight: .medium, design: .monospaced)) - .foregroundStyle(.tertiary) - } - .padding(.horizontal, 10) - .frame(height: GitChangesView.topBarHeight) - .background(Color(nsColor: settings.terminalBackgroundColor)) - .overlay(alignment: .bottom) { - Rectangle().fill(Color.primary.opacity(0.08)).frame(height: 1) - } + StackedDiffBody( + files: files, patches: patches, repoRoot: repoRoot, + settings: settings, onClose: onClose + ) } // MARK: List rail @@ -304,6 +277,185 @@ private struct PRFileDiffBody: View { } } +// MARK: - Stacked multi-file diff (narrow mode) + +/// Narrow mode's right section: GitHub's "Files changed" — each changed file is its own collapsible +/// card, stacked in one outer scroll. The card's header is real SwiftUI (so the path sits flush-left +/// and the header visually owns the diff beneath it, as one bordered unit); the diff body reuses the +/// shared `DiffTextPane` in embedded, content-sized mode, so the outer list — not each pane — scrolls. +private struct StackedDiffBody: View { + let files: [GitChange] + let patches: [String: String] + let repoRoot: String + @ObservedObject var settings: AppSettings + let onClose: () -> Void + + /// Paths of the files folded shut. + @State private var collapsed: Set = [] + + var body: some View { + ScrollView { + LazyVStack(spacing: 10) { + ForEach(files, id: \.path) { change in + FileDiffCard( + change: change, + patch: patches[change.path], + repoRoot: repoRoot, + settings: settings, + collapsed: collapsed.contains(change.path), + onToggle: { + if collapsed.contains(change.path) { collapsed.remove(change.path) } + else { collapsed.insert(change.path) } + }, + onClose: onClose + ) + } + } + .padding(10) + } + .background(Color(nsColor: settings.terminalBackgroundColor)) + .onExitCommand(perform: onClose) + } +} + +/// One file as a collapsible card: a flush-left header bar (chevron, status letter, path, `+/−`) over +/// the file's diff. Collapsed shows only the header; expanded lazily parses the patch and renders it +/// in an embedded `DiffTextPane` sized to its content, so the outer list owns the scroll. +private struct FileDiffCard: View { + let change: GitChange + let patch: String? + let repoRoot: String + @ObservedObject var settings: AppSettings + let collapsed: Bool + let onToggle: () -> Void + let onClose: () -> Void + + @Environment(\.colorScheme) private var colorScheme + @State private var rows: [DiffRow] = [] + @State private var document: DiffDocument? + @State private var styledLines: [Int: NSAttributedString] = [:] + @State private var expanded: Set = [] + @State private var contentHeight: CGFloat = 0 + @State private var loaded = false + + var body: some View { + VStack(spacing: 0) { + header + if !collapsed { + Rectangle().fill(Color.primary.opacity(0.08)).frame(height: 1) + diffBody + } + } + .background(Color(nsColor: settings.terminalBackgroundColor)) + .clipShape(RoundedRectangle(cornerRadius: 8, style: .continuous)) + .overlay( + RoundedRectangle(cornerRadius: 8, style: .continuous) + .stroke(Color.primary.opacity(0.12), lineWidth: 1) + ) + // Parse lazily — only when the card is (or becomes) expanded. + .task(id: collapsed) { + if !collapsed, !loaded { await load() } + } + } + + /// The title bar — the whole row toggles the fold; the disclosure chevron mirrors the state. + private var header: some View { + Button(action: onToggle) { + HStack(spacing: 8) { + Image(systemName: collapsed ? "chevron.right" : "chevron.down") + .font(.system(size: 10, weight: .semibold)) + .foregroundStyle(.secondary) + .frame(width: 11) + Text(change.status.letter) + .font(.system(size: 11, weight: .bold, design: .monospaced)) + .foregroundStyle(change.status.tint) + Text(change.path) + .font(.system(size: 12, weight: .medium)) + .lineLimit(1) + .truncationMode(.middle) + Spacer(minLength: 8) + HStack(spacing: 5) { + if change.additions > 0 { Text("+\(change.additions)").foregroundStyle(.green) } + if change.deletions > 0 { Text("−\(change.deletions)").foregroundStyle(.red) } + } + .font(.system(size: 11, weight: .medium, design: .monospaced)) + .fixedSize() + } + .padding(.horizontal, 10) + .frame(height: 34) + .frame(maxWidth: .infinity, alignment: .leading) + .contentShape(Rectangle()) + .background(Color.primary.opacity(0.05)) + } + .buttonStyle(.plain) + .help(change.path) + } + + @ViewBuilder + private var diffBody: some View { + if let document { + DiffTextPane( + document: document, + styled: styledLines, + font: settings.resolvedTerminalFont(), + backgroundColor: settings.terminalBackgroundColor, + numberColor: settings.gutterInk(for: colorScheme), + onExpand: { id in expanded.insert(id); rebuildDocument() }, + onWalk: { _ in false }, + onClose: onClose, + embedded: true, + onContentHeight: { contentHeight = $0 } + ) + .frame(height: max(contentHeight, 24)) + } else if !loaded { + ProgressView().controlSize(.small) + .frame(maxWidth: .infinity).frame(height: 44) + } else { + Text(change.isBinary + ? "Binary file — open it on GitHub to view." + : "This diff is too large to show here — open the file on GitHub.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.horizontal, 12) + .padding(.vertical, 10) + } + } + + private func load() async { + loaded = true + guard let patch, !patch.isEmpty else { rows = []; document = nil; return } + let parsed = await GitService.parseDiffText(patch) + rows = parsed + document = parsed.isEmpty + ? nil + : DiffDocument.build(rows: parsed, expanded: expanded, + codeFont: settings.resolvedTerminalFont()) + await buildStyled(parsed) + } + + private func rebuildDocument() { + document = DiffDocument.build(rows: rows, expanded: expanded, + codeFont: settings.resolvedTerminalFont()) + } + + private func buildStyled(_ parsed: [DiffRow]) async { + let url = URL(fileURLWithPath: repoRoot).appendingPathComponent(change.path) + guard let language = FileEditorView.highlightLanguage(for: url) else { return } + let code = parsed.filter { $0.kind != .hunk } + guard code.count <= 8000, code.reduce(0, { $0 + $1.text.count }) <= 600_000 else { return } + let styled = await DiffHighlighter.shared.styledLines( + newSide: code.filter { $0.kind == .context || $0.kind == .addition }, + oldSide: code.filter { $0.kind == .context || $0.kind == .deletion }, + language: language, + theme: colorScheme == .dark ? "xcode-dark" : "xcode", + font: settings.resolvedTerminalFont() + ) + guard !Task.isCancelled else { return } + styledLines = styled + } +} + // MARK: - File row /// A PR file row: the Changes list's visual language (status letter, name, dimmed diff --git a/Sources/termio/Issues/IssuesPanelModel.swift b/Sources/termio/Issues/IssuesPanelModel.swift index 5193e7af..29d41271 100644 --- a/Sources/termio/Issues/IssuesPanelModel.swift +++ b/Sources/termio/Issues/IssuesPanelModel.swift @@ -50,6 +50,11 @@ final class IssuesPanelModel: ObservableObject { var capabilities: IssueCapabilities? { provider?.capabilities } + /// Whether the one-time resolve-and-load has run for this model. The model is registered on + /// the store (see `TermioStore.registerIssuesModel`), whose `.task` re-fires on every remount + /// — so the initial git resolve + list pull must run once, not on each maximize toggle. + private var didStart = false + /// Entry point on appear: restore the Keychain token, resolve the binding /// from the origin remote, and load. func start() async { @@ -60,6 +65,8 @@ final class IssuesPanelModel: ObservableObject { phase = .disconnected return } + guard !didStart else { return } + didStart = true await resolveContainer() await loadList() } @@ -78,6 +85,7 @@ final class IssuesPanelModel: ObservableObject { let token = try await GitHubIssueAuth.waitForToken(code) GitHubIssueAuth.store(token: token) provider = GitHubIssueProvider(token: token) + didStart = true await resolveContainer() await loadList() } catch { @@ -92,6 +100,7 @@ final class IssuesPanelModel: ObservableObject { items = [] openItem = nil detail = nil + didStart = false phase = .disconnected errorMessage = nil recovery = nil @@ -171,6 +180,10 @@ final class IssuesPanelModel: ObservableObject { func loadDetail(for item: IssueSummary) async { guard let provider, let container else { return } + // Maximizing hoists the detail into the full-window host, which remounts the view and + // re-fires its `.task` — but the model outlives that. Skip the wipe-and-refetch when this + // exact item is already loaded, so a layout change costs no spinner or network round-trip. + if openItem?.number == item.number, detail != nil { return } // The model's own record of which item is open, independent of what drives the UI. openItem = item detail = nil