Skip to content

Commit 6ec52ab

Browse files
arzafranarzafran
authored
fix: split divider on dark themes (#26), browser downloads to ~/Downloads (#9), shellState dedup guard (#32) (#45)
* fix: lighten split divider on dark backgrounds so it stays visible (#26) On near-black terminal backgrounds the computed divider was darkened toward black and vanished. Lighten dark backgrounds with an additive-brightness helper instead; light backgrounds still darken. An explicit split-divider-color override always wins. Adds GhosttyConfig divider tests. * test: guard shellState dedup race in SocketFastPathState (#32) shouldPublishShellActivity must not record the state it reads, or a report that never applies (panel absent) suppresses the next identical report and the activity update is lost. The fix is already on main; this regression guard locks it in. * fix: auto-save browser downloads to ~/Downloads, Safari-style (#9) Replace the NSSavePanel prompt on download completion with a direct move to ~/Downloads, appending " 2", " 3", … on filename collisions like Safari.
1 parent dbf9d12 commit 6ec52ab

5 files changed

Lines changed: 154 additions & 20 deletions

File tree

‎GhosttyTabs.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
0F2C25F9170130F8DC09DD1B /* WorkspaceManualUnreadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D301919B10F22B8708E8883 /* WorkspaceManualUnreadTests.swift */; };
134134
CA39C0304FE351A21C372429 /* SidebarWidthPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE0171AF1F49F7547191CEE5 /* SidebarWidthPolicyTests.swift */; };
135135
8C4BBF2DEF6DF93F395A9EE7 /* TerminalControllerSocketSecurityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */; };
136+
C1A2B3C4D5E6F70800000004 /* TerminalControllerShellStateDedupTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1A2B3C4D5E6F70800000003 /* TerminalControllerShellStateDedupTests.swift */; };
136137
2BB56A710BB1FC50367E5BCF /* TabManagerSessionSnapshotTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 10D684CFFB8CDEF89CE2D9E1 /* TabManagerSessionSnapshotTests.swift */; };
137138
C1A2B3C4D5E6F70800000001 /* CmuxConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1A2B3C4D5E6F70800000002 /* CmuxConfigTests.swift */; };
138139
/* End PBXBuildFile section */
@@ -332,6 +333,7 @@
332333
1D301919B10F22B8708E8883 /* WorkspaceManualUnreadTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceManualUnreadTests.swift; sourceTree = "<group>"; };
333334
EE0171AF1F49F7547191CEE5 /* SidebarWidthPolicyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWidthPolicyTests.swift; sourceTree = "<group>"; };
334335
491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerSocketSecurityTests.swift; sourceTree = "<group>"; };
336+
C1A2B3C4D5E6F70800000003 /* TerminalControllerShellStateDedupTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalControllerShellStateDedupTests.swift; sourceTree = "<group>"; };
335337
10D684CFFB8CDEF89CE2D9E1 /* TabManagerSessionSnapshotTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabManagerSessionSnapshotTests.swift; sourceTree = "<group>"; };
336338
C1A2B3C4D5E6F70800000002 /* CmuxConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CmuxConfigTests.swift; sourceTree = "<group>"; };
337339
/* End PBXFileReference section */
@@ -613,6 +615,7 @@
613615
1D301919B10F22B8708E8883 /* WorkspaceManualUnreadTests.swift */,
614616
EE0171AF1F49F7547191CEE5 /* SidebarWidthPolicyTests.swift */,
615617
491751CE2321474474F27DCF /* TerminalControllerSocketSecurityTests.swift */,
618+
C1A2B3C4D5E6F70800000003 /* TerminalControllerShellStateDedupTests.swift */,
616619
10D684CFFB8CDEF89CE2D9E1 /* TabManagerSessionSnapshotTests.swift */,
617620
C1A2B3C4D5E6F70800000002 /* CmuxConfigTests.swift */,
618621
);
@@ -897,6 +900,7 @@
897900
0F2C25F9170130F8DC09DD1B /* WorkspaceManualUnreadTests.swift in Sources */,
898901
CA39C0304FE351A21C372429 /* SidebarWidthPolicyTests.swift in Sources */,
899902
8C4BBF2DEF6DF93F395A9EE7 /* TerminalControllerSocketSecurityTests.swift in Sources */,
903+
C1A2B3C4D5E6F70800000004 /* TerminalControllerShellStateDedupTests.swift in Sources */,
900904
2BB56A710BB1FC50367E5BCF /* TabManagerSessionSnapshotTests.swift in Sources */,
901905
C1A2B3C4D5E6F70800000001 /* CmuxConfigTests.swift in Sources */,
902906
);

‎Sources/GhosttyConfig.swift‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,14 @@ struct GhosttyConfig {
5353
return splitDividerColor
5454
}
5555

56+
// On light backgrounds a subtle darken reads as a divider. On dark/near-black
57+
// backgrounds, darkening pushes toward black and the divider disappears — lighten
58+
// instead so the divider stays visible. Users can still override via
59+
// `split-divider-color`.
5660
let isLightBackground = backgroundColor.isLightColor
57-
return backgroundColor.darken(by: isLightBackground ? 0.08 : 0.4)
61+
return isLightBackground
62+
? backgroundColor.darken(by: 0.08)
63+
: backgroundColor.lighten(by: 0.18)
5864
}
5965

6066
static func load(
@@ -589,4 +595,20 @@ extension NSColor {
589595
alpha: a
590596
)
591597
}
598+
599+
/// Additively raises brightness so the result stays visible even on pure black
600+
/// (where multiplicative scaling would have no effect).
601+
func lighten(by amount: CGFloat) -> NSColor {
602+
var h: CGFloat = 0
603+
var s: CGFloat = 0
604+
var b: CGFloat = 0
605+
var a: CGFloat = 0
606+
getHue(&h, saturation: &s, brightness: &b, alpha: &a)
607+
return NSColor(
608+
hue: h,
609+
saturation: s,
610+
brightness: min(b + amount, 1),
611+
alpha: a
612+
)
613+
}
592614
}

‎Sources/Panels/BrowserPanel.swift‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5853,6 +5853,27 @@ class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
58535853
return safe.isEmpty ? "download" : safe
58545854
}
58555855

5856+
/// Resolves a non-colliding destination in ~/Downloads, appending " 2", " 3", … like Safari.
5857+
static func uniqueDownloadsURL(for filename: String) -> URL {
5858+
let fileManager = FileManager.default
5859+
let downloads = fileManager.urls(for: .downloadsDirectory, in: .userDomainMask).first
5860+
?? fileManager.homeDirectoryForCurrentUser.appendingPathComponent("Downloads", isDirectory: true)
5861+
try? fileManager.createDirectory(at: downloads, withIntermediateDirectories: true)
5862+
5863+
var candidate = downloads.appendingPathComponent(filename, isDirectory: false)
5864+
guard fileManager.fileExists(atPath: candidate.path) else { return candidate }
5865+
5866+
let base = (filename as NSString).deletingPathExtension
5867+
let ext = (filename as NSString).pathExtension
5868+
var counter = 2
5869+
repeat {
5870+
let name = ext.isEmpty ? "\(base) \(counter)" : "\(base) \(counter).\(ext)"
5871+
candidate = downloads.appendingPathComponent(name, isDirectory: false)
5872+
counter += 1
5873+
} while fileManager.fileExists(atPath: candidate.path)
5874+
return candidate
5875+
}
5876+
58565877
private func storeState(_ state: DownloadState, for download: WKDownload) {
58575878
activeDownloadsLock.lock()
58585879
activeDownloads[ObjectIdentifier(download)] = state
@@ -5908,27 +5929,16 @@ class BrowserDownloadDelegate: NSObject, WKDownloadDelegate {
59085929
#endif
59095930
NSLog("BrowserPanel download finished: %@", info.suggestedFilename)
59105931

5911-
// Show NSSavePanel on the next runloop iteration (safe context).
5932+
// #9: auto-save to ~/Downloads (Safari-style) instead of prompting with a save panel.
59125933
DispatchQueue.main.async {
59135934
self.onDownloadReadyToSave?()
5914-
let savePanel = NSSavePanel()
5915-
savePanel.nameFieldStringValue = info.suggestedFilename
5916-
savePanel.canCreateDirectories = true
5917-
savePanel.directoryURL = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask).first
5918-
5919-
savePanel.begin { result in
5920-
guard result == .OK, let destURL = savePanel.url else {
5921-
try? FileManager.default.removeItem(at: info.tempURL)
5922-
return
5923-
}
5924-
do {
5925-
try? FileManager.default.removeItem(at: destURL)
5926-
try FileManager.default.moveItem(at: info.tempURL, to: destURL)
5927-
NSLog("BrowserPanel download saved: %@", destURL.path)
5928-
} catch {
5929-
NSLog("BrowserPanel download move failed: %@", error.localizedDescription)
5930-
try? FileManager.default.removeItem(at: info.tempURL)
5931-
}
5935+
let destURL = Self.uniqueDownloadsURL(for: info.suggestedFilename)
5936+
do {
5937+
try FileManager.default.moveItem(at: info.tempURL, to: destURL)
5938+
NSLog("BrowserPanel download saved: %@", destURL.path)
5939+
} catch {
5940+
NSLog("BrowserPanel download move failed: %@", error.localizedDescription)
5941+
try? FileManager.default.removeItem(at: info.tempURL)
59325942
}
59335943
}
59345944
}

‎cmuxTests/GhosttyConfigTests.swift‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,43 @@ final class GhosttyConfigTests: XCTestCase {
7979
XCTAssertTrue(candidates.contains("iTerm2 Solarized Dark"))
8080
}
8181

82+
// #26: on near-black backgrounds the divider must be lightened to stay visible,
83+
// not darkened toward black.
84+
func testSplitDividerLightensOnNearBlackBackground() {
85+
var config = GhosttyConfig()
86+
config.splitDividerColor = nil
87+
config.backgroundColor = NSColor(hex: "#0a0a0a")!
88+
89+
let divider = config.resolvedSplitDividerColor
90+
XCTAssertGreaterThan(
91+
divider.luminance,
92+
config.backgroundColor.luminance + 0.05,
93+
"Divider on a near-black background should be clearly lighter than the background"
94+
)
95+
}
96+
97+
func testSplitDividerDarkensOnLightBackground() {
98+
var config = GhosttyConfig()
99+
config.splitDividerColor = nil
100+
config.backgroundColor = NSColor(hex: "#fafafa")!
101+
102+
let divider = config.resolvedSplitDividerColor
103+
XCTAssertLessThan(
104+
divider.luminance,
105+
config.backgroundColor.luminance,
106+
"Divider on a light background should be darker than the background"
107+
)
108+
}
109+
110+
func testSplitDividerHonorsExplicitOverride() {
111+
var config = GhosttyConfig()
112+
let override = NSColor(hex: "#ff0000")!
113+
config.splitDividerColor = override
114+
config.backgroundColor = NSColor(hex: "#0a0a0a")!
115+
116+
XCTAssertEqual(config.resolvedSplitDividerColor, override)
117+
}
118+
82119
func testThemeSearchPathsIncludeXDGDataDirsThemes() {
83120
let pathA = "/tmp/programa-theme-a"
84121
let pathB = "/tmp/programa-theme-b"

‎cmuxTests/TerminalControllerShellStateDedupTests.swift‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import XCTest
2+
3+
#if canImport(Programa_DEV)
4+
@testable import Programa_DEV
5+
#elseif canImport(Programa)
6+
@testable import Programa
7+
#endif
8+
9+
/// Regression guard for the #6618 shellState dedup race.
10+
///
11+
/// `SocketFastPathState.shouldPublishShellActivity` must NOT record the state it
12+
/// reads. Recording happens only via `recordShellActivity`, called after the
13+
/// update is confirmed applied on the main thread. The old implementation wrote
14+
/// on read, so a report that was never applied (panel absent) would suppress the
15+
/// next identical report — losing the activity update permanently.
16+
final class TerminalControllerShellStateDedupTests: XCTestCase {
17+
func testShouldPublishDoesNotSuppressUntilActivityRecorded() {
18+
let state = TerminalController.SocketFastPathState()
19+
let workspaceId = UUID()
20+
let panelId = UUID()
21+
let activity: Workspace.PanelShellActivityState = .commandRunning
22+
23+
// 1. First observation of a state is always worth publishing.
24+
XCTAssertTrue(
25+
state.shouldPublishShellActivity(
26+
workspaceId: workspaceId,
27+
panelId: panelId,
28+
state: activity
29+
),
30+
"First state report should be publishable"
31+
)
32+
33+
// 2. Simulate the apply failing (panel absent): recordShellActivity is NOT called.
34+
35+
// 3. The identical state must STILL be publishable, because nothing was
36+
// recorded — this is the regression. Write-on-read would return false here.
37+
XCTAssertTrue(
38+
state.shouldPublishShellActivity(
39+
workspaceId: workspaceId,
40+
panelId: panelId,
41+
state: activity
42+
),
43+
"Identical state must remain publishable until it is recorded as applied"
44+
)
45+
46+
// 4. After recording the applied state, the identical report is deduped.
47+
state.recordShellActivity(
48+
workspaceId: workspaceId,
49+
panelId: panelId,
50+
state: activity
51+
)
52+
XCTAssertFalse(
53+
state.shouldPublishShellActivity(
54+
workspaceId: workspaceId,
55+
panelId: panelId,
56+
state: activity
57+
),
58+
"Once recorded, an identical state should be deduped"
59+
)
60+
}
61+
}

0 commit comments

Comments
 (0)