Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Package.resolved

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ let package = Package(
.executable(name: "swift-cli-example", targets: ["SwiftCLIExample"])
],
dependencies: [
.package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.4.0")
.package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.4.0"),
.package(url: "https://github.com/apple/swift-atomics.git", from: "1.2.0")
],
targets: [
// MARK: - Foundation Layer
Expand All @@ -38,7 +39,10 @@ let package = Package(
/// Low-level terminal operations: I/O, raw mode, capabilities
.target(
name: "TerminalCore",
dependencies: ["ANSI"]
dependencies: [
"ANSI",
.product(name: "Atomics", package: "swift-atomics")
]
),

// MARK: - Feature Packages
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftCLIExample/SwiftCLIExample.swift
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ func demoTerminalInfo(_ terminal: Terminal) async throws {
await terminal.writeLine("")

let caps = await terminal.capabilities
let size = await terminal.size
let size = await terminal.refreshSize()
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change from terminal.size to terminal.refreshSize() is unnecessary for this demo. The size property is already kept up-to-date by checkResize() calls in the event loop. Using refreshSize() here forces an extra system call. Consider reverting to await terminal.size unless there's a specific reason to force a refresh at this point.

Suggested change
let size = await terminal.refreshSize()
let size = await terminal.size

Copilot uses AI. Check for mistakes.

await terminal.render(Box(
"""
Expand Down
32 changes: 14 additions & 18 deletions Sources/TerminalCore/Terminal.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import ANSI
import Atomics
import Dispatch

#if canImport(Darwin)
Expand All @@ -7,6 +8,9 @@ import Darwin
import Glibc
#endif

/// Thread-safe flag for SIGWINCH signal (accessible from signal handler without actor isolation)
private let _resizeSignalReceived = ManagedAtomic<Bool>(false)

/// The central actor for terminal operations.
///
/// `Terminal` provides thread-safe access to terminal capabilities including
Expand Down Expand Up @@ -61,9 +65,6 @@ public actor Terminal {
/// Signal source for SIGWINCH (terminal resize)
private var resizeSignalSource: DispatchSourceSignal?

/// Whether a resize event has occurred
public private(set) var resizeOccurred: Bool = false

// MARK: - Initialization

/// Create a new terminal instance with default configuration.
Expand Down Expand Up @@ -418,12 +419,9 @@ public actor Terminal {
// Create dispatch source for SIGWINCH
let source = DispatchSource.makeSignalSource(signal: SIGWINCH, queue: .main)
source.setEventHandler {
// Set a flag that can be checked in the event loop
// We can't safely call actor methods from here, so we just
// trigger the flag synchronously. The event loop will check it.
Task { @MainActor in
await Terminal.shared.handleResize()
}
// Set atomic flag directly - no Task needed
// This allows the flag to be set even when poll(2) is blocking
_resizeSignalReceived.store(true, ordering: .relaxed)
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using .relaxed memory ordering may not guarantee visibility of the flag update across threads. Consider using .sequentiallyConsistent or at minimum .acquiring/.releasing ordering to ensure proper synchronization between the signal handler and the actor thread checking the flag in checkResize().

Copilot uses AI. Check for mistakes.
}
source.resume()
resizeSignalSource = source
Expand All @@ -436,17 +434,15 @@ public actor Terminal {
resizeSignalSource = nil
}

/// Handle a resize event.
private func handleResize() {
size = TerminalSize.detect(fd: outputFD)
resizeOccurred = true
}

/// Check and clear the resize flag.
/// Returns true if a resize occurred since last check.
public func checkResize() -> Bool {
let didResize = resizeOccurred
resizeOccurred = false
return didResize
// Atomically check and clear the flag
if _resizeSignalReceived.exchange(false, ordering: .relaxed) {
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .relaxed memory ordering may not guarantee visibility of the flag set in the signal handler. Consider using .sequentiallyConsistent or .acquiring ordering to ensure the flag update from the signal handler is visible when checking here.

Copilot uses AI. Check for mistakes.
// Signal was received - refresh the terminal size
size = TerminalSize.detect(fd: outputFD)
return true
}
return false
}
Comment on lines 437 to 447
Copy link

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new atomic-based resize handling logic lacks test coverage. Consider adding tests to verify: 1) checkResize() returns false when no signal is received, 2) checkResize() returns true and updates size when the flag is set, and 3) subsequent calls to checkResize() return false after clearing the flag.

Copilot uses AI. Check for mistakes.
}