Merge pull request #2405 from manaflow-ai/issue-2388-sidebar-layout-regression
Fix duplicate sidebar git metadata publishes
This commit is contained in:
commit
6e6a2c95b5
3 changed files with 206 additions and 17 deletions
|
|
@ -11163,7 +11163,7 @@ enum SidebarTrailingAccessoryWidthPolicy {
|
||||||
// Reactive workspace state inside the row must not rely on parent diffs alone:
|
// Reactive workspace state inside the row must not rely on parent diffs alone:
|
||||||
// `.equatable()` can otherwise leave sidebar badges/details stale until an
|
// `.equatable()` can otherwise leave sidebar badges/details stale until an
|
||||||
// unrelated parent change sneaks through. Keep the workspace reference plain
|
// unrelated parent change sneaks through. Keep the workspace reference plain
|
||||||
// and bridge its objectWillChange into local state instead.
|
// and bridge only sidebar-visible workspace changes into local state.
|
||||||
// Do NOT add @EnvironmentObject or new @Binding without updating ==.
|
// Do NOT add @EnvironmentObject or new @Binding without updating ==.
|
||||||
// Do NOT remove .equatable() from the ForEach call site in VerticalTabsSidebar.
|
// Do NOT remove .equatable() from the ForEach call site in VerticalTabsSidebar.
|
||||||
private struct TabItemView: View, Equatable {
|
private struct TabItemView: View, Equatable {
|
||||||
|
|
@ -11795,7 +11795,7 @@ private struct TabItemView: View, Equatable {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
.onReceive(
|
.onReceive(
|
||||||
tab.objectWillChange
|
tab.sidebarObservationPublisher
|
||||||
.receive(on: RunLoop.main)
|
.receive(on: RunLoop.main)
|
||||||
// Prompt-time sidebar telemetry can arrive as a short burst
|
// Prompt-time sidebar telemetry can arrive as a short burst
|
||||||
// (pwd, branch, PR, shell state). Coalesce that burst so the
|
// (pwd, branch, PR, shell state). Coalesce that burst so the
|
||||||
|
|
|
||||||
|
|
@ -112,7 +112,7 @@ func cmuxInheritedSurfaceConfig(
|
||||||
return config
|
return config
|
||||||
}
|
}
|
||||||
|
|
||||||
struct SidebarStatusEntry {
|
struct SidebarStatusEntry: Equatable {
|
||||||
let key: String
|
let key: String
|
||||||
let value: String
|
let value: String
|
||||||
let icon: String?
|
let icon: String?
|
||||||
|
|
@ -143,7 +143,7 @@ struct SidebarStatusEntry {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
struct SidebarMetadataBlock {
|
struct SidebarMetadataBlock: Equatable {
|
||||||
let key: String
|
let key: String
|
||||||
let markdown: String
|
let markdown: String
|
||||||
let priority: Int
|
let priority: Int
|
||||||
|
|
@ -4896,23 +4896,31 @@ enum SidebarLogLevel: String {
|
||||||
case error
|
case error
|
||||||
}
|
}
|
||||||
|
|
||||||
struct SidebarLogEntry {
|
struct SidebarLogEntry: Equatable {
|
||||||
let message: String
|
let message: String
|
||||||
let level: SidebarLogLevel
|
let level: SidebarLogLevel
|
||||||
let source: String?
|
let source: String?
|
||||||
let timestamp: Date
|
let timestamp: Date
|
||||||
}
|
}
|
||||||
|
|
||||||
struct SidebarProgressState {
|
struct SidebarProgressState: Equatable {
|
||||||
let value: Double
|
let value: Double
|
||||||
let label: String?
|
let label: String?
|
||||||
}
|
}
|
||||||
|
|
||||||
struct SidebarGitBranchState {
|
struct SidebarGitBranchState: Equatable {
|
||||||
let branch: String
|
let branch: String
|
||||||
let isDirty: Bool
|
let isDirty: Bool
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private struct SidebarPanelObservationState: Equatable {
|
||||||
|
let panelIds: [UUID]
|
||||||
|
|
||||||
|
init(panels: [UUID: any Panel]) {
|
||||||
|
panelIds = panels.keys.sorted { $0.uuidString < $1.uuidString }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
enum WorkspaceRemoteConnectionState: String {
|
enum WorkspaceRemoteConnectionState: String {
|
||||||
case disconnected
|
case disconnected
|
||||||
case connecting
|
case connecting
|
||||||
|
|
@ -5583,6 +5591,47 @@ final class Workspace: Identifiable, ObservableObject {
|
||||||
var agentPIDs: [String: pid_t] = [:]
|
var agentPIDs: [String: pid_t] = [:]
|
||||||
private var restoredTerminalScrollbackByPanelId: [UUID: String] = [:]
|
private var restoredTerminalScrollbackByPanelId: [UUID: String] = [:]
|
||||||
|
|
||||||
|
private func sidebarObservationSignal<Value: Equatable>(
|
||||||
|
_ publisher: Published<Value>.Publisher
|
||||||
|
) -> AnyPublisher<Void, Never> {
|
||||||
|
publisher
|
||||||
|
.dropFirst()
|
||||||
|
.removeDuplicates()
|
||||||
|
.map { _ in () }
|
||||||
|
.eraseToAnyPublisher()
|
||||||
|
}
|
||||||
|
|
||||||
|
lazy var sidebarObservationPublisher: AnyPublisher<Void, Never> = {
|
||||||
|
let publishers: [AnyPublisher<Void, Never>] = [
|
||||||
|
sidebarObservationSignal($title),
|
||||||
|
sidebarObservationSignal($isPinned),
|
||||||
|
sidebarObservationSignal($customColor),
|
||||||
|
sidebarObservationSignal($currentDirectory),
|
||||||
|
$panels
|
||||||
|
.map(SidebarPanelObservationState.init)
|
||||||
|
.dropFirst()
|
||||||
|
.removeDuplicates()
|
||||||
|
.map { _ in () }
|
||||||
|
.eraseToAnyPublisher(),
|
||||||
|
sidebarObservationSignal($panelDirectories),
|
||||||
|
sidebarObservationSignal($statusEntries),
|
||||||
|
sidebarObservationSignal($metadataBlocks),
|
||||||
|
sidebarObservationSignal($logEntries),
|
||||||
|
sidebarObservationSignal($progress),
|
||||||
|
sidebarObservationSignal($gitBranch),
|
||||||
|
sidebarObservationSignal($panelGitBranches),
|
||||||
|
sidebarObservationSignal($pullRequest),
|
||||||
|
sidebarObservationSignal($panelPullRequests),
|
||||||
|
sidebarObservationSignal($remoteConfiguration),
|
||||||
|
sidebarObservationSignal($remoteConnectionState),
|
||||||
|
sidebarObservationSignal($remoteConnectionDetail),
|
||||||
|
sidebarObservationSignal($activeRemoteTerminalSessionCount),
|
||||||
|
sidebarObservationSignal($listeningPorts),
|
||||||
|
]
|
||||||
|
|
||||||
|
return Publishers.MergeMany(publishers).eraseToAnyPublisher()
|
||||||
|
}()
|
||||||
|
|
||||||
private static func isProxyOnlyRemoteError(_ detail: String) -> Bool {
|
private static func isProxyOnlyRemoteError(_ detail: String) -> Bool {
|
||||||
let lowered = detail.lowercased()
|
let lowered = detail.lowercased()
|
||||||
return lowered.contains("remote proxy")
|
return lowered.contains("remote proxy")
|
||||||
|
|
@ -6449,22 +6498,32 @@ final class Workspace: Identifiable, ObservableObject {
|
||||||
panelGitBranches[panelId] = state
|
panelGitBranches[panelId] = state
|
||||||
}
|
}
|
||||||
if branchChanged {
|
if branchChanged {
|
||||||
panelPullRequests.removeValue(forKey: panelId)
|
if panelPullRequests[panelId] != nil {
|
||||||
if panelId == focusedPanelId {
|
panelPullRequests.removeValue(forKey: panelId)
|
||||||
|
}
|
||||||
|
if panelId == focusedPanelId, pullRequest != nil {
|
||||||
pullRequest = nil
|
pullRequest = nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if panelId == focusedPanelId {
|
if panelId == focusedPanelId, gitBranch != state {
|
||||||
gitBranch = state
|
gitBranch = state
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func clearPanelGitBranch(panelId: UUID) {
|
func clearPanelGitBranch(panelId: UUID) {
|
||||||
panelGitBranches.removeValue(forKey: panelId)
|
if panelGitBranches[panelId] != nil {
|
||||||
panelPullRequests.removeValue(forKey: panelId)
|
panelGitBranches.removeValue(forKey: panelId)
|
||||||
|
}
|
||||||
|
if panelPullRequests[panelId] != nil {
|
||||||
|
panelPullRequests.removeValue(forKey: panelId)
|
||||||
|
}
|
||||||
if panelId == focusedPanelId {
|
if panelId == focusedPanelId {
|
||||||
gitBranch = nil
|
if gitBranch != nil {
|
||||||
pullRequest = nil
|
gitBranch = nil
|
||||||
|
}
|
||||||
|
if pullRequest != nil {
|
||||||
|
pullRequest = nil
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -6520,14 +6579,16 @@ final class Workspace: Identifiable, ObservableObject {
|
||||||
if existing != state {
|
if existing != state {
|
||||||
panelPullRequests[panelId] = state
|
panelPullRequests[panelId] = state
|
||||||
}
|
}
|
||||||
if panelId == focusedPanelId {
|
if panelId == focusedPanelId, pullRequest != state {
|
||||||
pullRequest = state
|
pullRequest = state
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func clearPanelPullRequest(panelId: UUID) {
|
func clearPanelPullRequest(panelId: UUID) {
|
||||||
panelPullRequests.removeValue(forKey: panelId)
|
if panelPullRequests[panelId] != nil {
|
||||||
if panelId == focusedPanelId {
|
panelPullRequests.removeValue(forKey: panelId)
|
||||||
|
}
|
||||||
|
if panelId == focusedPanelId, pullRequest != nil {
|
||||||
pullRequest = nil
|
pullRequest = nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ import WebKit
|
||||||
import ObjectiveC.runtime
|
import ObjectiveC.runtime
|
||||||
import Bonsplit
|
import Bonsplit
|
||||||
import UserNotifications
|
import UserNotifications
|
||||||
|
import Combine
|
||||||
|
|
||||||
#if canImport(cmux_DEV)
|
#if canImport(cmux_DEV)
|
||||||
@testable import cmux_DEV
|
@testable import cmux_DEV
|
||||||
|
|
@ -2111,6 +2112,133 @@ final class WorkspacePanelGitBranchTests: XCTestCase {
|
||||||
XCTAssertEqual(ordered.map(\.isDirty), [false, true])
|
XCTAssertEqual(ordered.map(\.isDirty), [false, true])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func testUpdatingFocusedPanelGitBranchWithSameStateDoesNotRepublishWorkspace() {
|
||||||
|
let workspace = Workspace()
|
||||||
|
guard let panelId = workspace.focusedPanelId else {
|
||||||
|
XCTFail("Expected initial focused panel")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
var publishCount = 0
|
||||||
|
let cancellable = workspace.objectWillChange.sink { _ in
|
||||||
|
publishCount += 1
|
||||||
|
}
|
||||||
|
defer { cancellable.cancel() }
|
||||||
|
|
||||||
|
workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false)
|
||||||
|
let baselinePublishCount = publishCount
|
||||||
|
|
||||||
|
XCTAssertGreaterThan(
|
||||||
|
baselinePublishCount,
|
||||||
|
0,
|
||||||
|
"Expected the first focused branch update to publish workspace changes"
|
||||||
|
)
|
||||||
|
|
||||||
|
workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false)
|
||||||
|
|
||||||
|
XCTAssertEqual(
|
||||||
|
publishCount,
|
||||||
|
baselinePublishCount,
|
||||||
|
"Expected identical focused branch refreshes to avoid extra workspace publishes"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func testUpdatingFocusedPanelPullRequestWithSameStateDoesNotRepublishWorkspace() {
|
||||||
|
let workspace = Workspace()
|
||||||
|
guard let panelId = workspace.focusedPanelId else {
|
||||||
|
XCTFail("Expected initial focused panel")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
workspace.updatePanelGitBranch(panelId: panelId, branch: "feature/sidebar-pr", isDirty: false)
|
||||||
|
|
||||||
|
var publishCount = 0
|
||||||
|
let cancellable = workspace.objectWillChange.sink { _ in
|
||||||
|
publishCount += 1
|
||||||
|
}
|
||||||
|
defer { cancellable.cancel() }
|
||||||
|
|
||||||
|
let pullRequestURL = URL(string: "https://github.com/manaflow-ai/cmux/pull/2388")!
|
||||||
|
workspace.updatePanelPullRequest(
|
||||||
|
panelId: panelId,
|
||||||
|
number: 2388,
|
||||||
|
label: "PR",
|
||||||
|
url: pullRequestURL,
|
||||||
|
status: .open,
|
||||||
|
branch: "feature/sidebar-pr"
|
||||||
|
)
|
||||||
|
let baselinePublishCount = publishCount
|
||||||
|
|
||||||
|
XCTAssertGreaterThan(
|
||||||
|
baselinePublishCount,
|
||||||
|
0,
|
||||||
|
"Expected the first focused pull request update to publish workspace changes"
|
||||||
|
)
|
||||||
|
|
||||||
|
workspace.updatePanelPullRequest(
|
||||||
|
panelId: panelId,
|
||||||
|
number: 2388,
|
||||||
|
label: "PR",
|
||||||
|
url: pullRequestURL,
|
||||||
|
status: .open,
|
||||||
|
branch: "feature/sidebar-pr"
|
||||||
|
)
|
||||||
|
|
||||||
|
XCTAssertEqual(
|
||||||
|
publishCount,
|
||||||
|
baselinePublishCount,
|
||||||
|
"Expected identical focused pull request refreshes to avoid extra workspace publishes"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func testSidebarObservationPublisherEmitsForFocusedGitBranchChangesOnlyOncePerState() {
|
||||||
|
let workspace = Workspace()
|
||||||
|
guard let panelId = workspace.focusedPanelId else {
|
||||||
|
XCTFail("Expected initial focused panel")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
var publishCount = 0
|
||||||
|
let cancellable = workspace.sidebarObservationPublisher.sink {
|
||||||
|
publishCount += 1
|
||||||
|
}
|
||||||
|
defer { cancellable.cancel() }
|
||||||
|
|
||||||
|
workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false)
|
||||||
|
let baselinePublishCount = publishCount
|
||||||
|
XCTAssertGreaterThan(
|
||||||
|
baselinePublishCount,
|
||||||
|
0,
|
||||||
|
"Expected focused git branch updates to invalidate sidebar rows"
|
||||||
|
)
|
||||||
|
|
||||||
|
workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false)
|
||||||
|
XCTAssertEqual(
|
||||||
|
publishCount,
|
||||||
|
baselinePublishCount,
|
||||||
|
"Expected identical git metadata refreshes to be ignored by sidebar rows"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func testSidebarObservationPublisherIgnoresRemoteHeartbeatOnlyChanges() {
|
||||||
|
let workspace = Workspace()
|
||||||
|
|
||||||
|
var publishCount = 0
|
||||||
|
let cancellable = workspace.sidebarObservationPublisher.sink {
|
||||||
|
publishCount += 1
|
||||||
|
}
|
||||||
|
defer { cancellable.cancel() }
|
||||||
|
|
||||||
|
workspace.remoteHeartbeatCount = 1
|
||||||
|
workspace.remoteLastHeartbeatAt = Date()
|
||||||
|
|
||||||
|
XCTAssertEqual(
|
||||||
|
publishCount,
|
||||||
|
0,
|
||||||
|
"Expected non-visible remote heartbeat updates to avoid invalidating sidebar rows"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
@MainActor
|
@MainActor
|
||||||
func testSidebarPullRequestsTrackFocusedPanelOnly() {
|
func testSidebarPullRequestsTrackFocusedPanelOnly() {
|
||||||
let workspace = Workspace()
|
let workspace = Workspace()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue