-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Decouple iOS primary terminal scroll #6081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
2c43d31
eee05fb
25eb92e
2438ab6
171d624
8e4636f
a8b2ead
a1697cc
c156503
f82bcbf
aff8198
b6e9f97
f73e2c3
415b28b
c617afa
d0797e3
ed18dda
6b54974
a7c2a20
e05ca47
c3b8822
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,6 +137,7 @@ struct GhosttySurfaceRepresentable: UIViewRepresentable { | |
| for await chunk in store.terminalOutputStream(surfaceID: surfaceID) { | ||
| guard !Task.isCancelled else { return } | ||
| guard let surfaceView else { return } | ||
| surfaceView.applyTerminalOutputMetadata(activeScreen: chunk.activeScreen) | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a render-grid chunk switches screens, this updates Useful? React with 👍 / 👎. |
||
| await surfaceView.processOutputAndWait(chunk.data) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| store.terminalOutputDidProcess( | ||
| surfaceID: surfaceID, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,37 +8,6 @@ import UIKit | |
|
|
||
| private let log = Logger(subsystem: "ai.manaflow.cmux.ios", category: "ghostty.surface") | ||
|
|
||
| // lint:allow namespace-enum — file-local DEBUG input-trace logger on the off-limits typing-latency render path; type reshape deferred to the GhosttySurfaceView UI-god-object split wave. | ||
| enum TerminalInputDebugLog { | ||
| private static let isEnabled = ProcessInfo.processInfo.environment["CMUX_INPUT_DEBUG"] == "1" | ||
| private static let logger = Logger(subsystem: "ai.manaflow.cmux.ios", category: "ghostty.input") | ||
|
|
||
| static func log(_ message: String) { | ||
| #if DEBUG | ||
| if ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil { | ||
| return | ||
| } | ||
| #endif | ||
| guard isEnabled else { return } | ||
| logger.debug("input: \(message, privacy: .public)") | ||
| } | ||
|
|
||
| static func textSummary(_ text: String) -> String { | ||
| let summary = String(reflecting: text) | ||
| guard summary.count > 96 else { return summary } | ||
| return "\(summary.prefix(96))..." | ||
| } | ||
|
|
||
| static func dataSummary(_ data: Data) -> String { | ||
| let prefix = data.prefix(32) | ||
| let prefixData = Data(prefix) | ||
| let hex = prefix.map { String(format: "%02X", $0) }.joined(separator: " ") | ||
| let utf8 = String(data: prefixData, encoding: .utf8) ?? "<non-utf8>" | ||
| let suffix = data.count > prefix.count ? " ..." : "" | ||
| return "len=\(data.count) hex=\(hex)\(suffix) utf8=\(textSummary(utf8))" | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| public protocol GhosttySurfaceViewDelegate: AnyObject { | ||
| func ghosttySurfaceView(_ surfaceView: GhosttySurfaceView, didProduceInput data: Data) | ||
|
|
@@ -622,6 +591,8 @@ public final class GhosttySurfaceView: UIView, TerminalSurfaceHosting { | |
| private var lastAppliedContentScale: CGFloat = 0 | ||
| private var surfaceHasReceivedOutput: Bool = false | ||
| private var shouldScrollInitialOutputToBottom = true | ||
| private var activeScreen: MobileTerminalRenderGridFrame.Screen = .primary | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| private let scrollForwardingPolicy = MobileTerminalScrollForwardingPolicy() | ||
| /// Serial background queue for `ghostty_surface_process_output`, which | ||
|
Comment on lines
+605
to
611
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle unknown screen metadata as “forward” to preserve raw-byte fallback behavior. Line 625 initializes 💡 Suggested fix- private var activeScreen: MobileTerminalRenderGridFrame.Screen = .primary
+ private var activeScreen: MobileTerminalRenderGridFrame.Screen?
@@
- guard scrollForwardingPolicy.shouldForwardToHost(activeScreen: activeScreen) else {
- return
- }
+ if let activeScreen,
+ !scrollForwardingPolicy.shouldForwardToHost(activeScreen: activeScreen) {
+ return
+ }
@@
public func applyTerminalOutputMetadata(
activeScreen: MobileTerminalRenderGridFrame.Screen?
) {
guard let activeScreen else { return }
self.activeScreen = activeScreen
}Also applies to: 1816-1819, 2124-2129 🤖 Prompt for AI Agents |
||
| /// blocks on libghostty's internal renderer/IO futex. Running it on the | ||
| /// main thread hangs the app until the scene-update watchdog kills it. | ||
|
|
@@ -1811,6 +1782,9 @@ public final class GhosttySurfaceView: UIView, TerminalSurfaceHosting { | |
| let cell = pendingScrollCell | ||
| pendingScrollLines = 0 | ||
| applyLocalScrollbackScroll(lines: lines, col: cell.col, row: cell.row) | ||
| guard scrollForwardingPolicy.shouldForwardToHost(activeScreen: activeScreen) else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Here Useful? React with 👍 / 👎. |
||
| return | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| delegate?.ghosttySurfaceView(self, didScrollLines: lines, atCol: cell.col, row: cell.row) | ||
| } | ||
|
|
||
|
|
@@ -2109,6 +2083,20 @@ public final class GhosttySurfaceView: UIView, TerminalSurfaceHosting { | |
| processOutput(data, completion: nil) | ||
| } | ||
|
|
||
| /// Applies metadata attached to the next terminal output chunk. | ||
| /// | ||
| /// Render-grid output carries the authoritative active screen, which lets | ||
| /// local scrollback stay phone-local on the primary screen while alternate | ||
| /// screen TUIs still receive host mouse-wheel events. | ||
| /// - Parameter activeScreen: The active screen from the render-grid frame, | ||
| /// or `nil` for raw byte fallback chunks. | ||
| public func applyTerminalOutputMetadata( | ||
| activeScreen: MobileTerminalRenderGridFrame.Screen? | ||
| ) { | ||
| guard let activeScreen else { return } | ||
| self.activeScreen = activeScreen | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| /// Process terminal output and return after the output has been applied. | ||
| /// | ||
| /// The call still performs libghostty output processing on the serial | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| #if canImport(UIKit) | ||
| import Foundation | ||
| import OSLog | ||
|
|
||
| // lint:allow namespace-enum — DEBUG input-trace logger on the off-limits typing-latency render path; type reshape deferred to the GhosttySurfaceView UI-god-object split wave. | ||
| enum TerminalInputDebugLog { | ||
| private static let isEnabled = ProcessInfo.processInfo.environment["CMUX_INPUT_DEBUG"] == "1" | ||
| private static let logger = Logger(subsystem: "ai.manaflow.cmux.ios", category: "ghostty.input") | ||
|
|
||
| static func log(_ message: String) { | ||
| #if DEBUG | ||
| if ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil { | ||
| return | ||
| } | ||
| #endif | ||
| guard isEnabled else { return } | ||
| logger.debug("input: \(message, privacy: .public)") | ||
|
Comment on lines
+10
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make debug logging lazy to avoid per-keystroke overhead when disabled. On Line 10, Proposed fix- static func log(_ message: String) {
- `#if` DEBUG
- if ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil {
- return
- }
- `#endif`
- guard isEnabled else { return }
- logger.debug("input: \(message, privacy: .public)")
- }
+ static func log(_ message: `@autoclosure` () -> String) {
+ guard isEnabled else { return }
+ `#if` DEBUG
+ if ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] != nil {
+ return
+ }
+ `#endif`
+ logger.debug("input: \(message(), privacy: .public)")
+ }As per coding guidelines, typing-latency-sensitive paths must avoid adding unnecessary per-event work, and this helper is used on terminal input events. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| static func textSummary(_ text: String) -> String { | ||
| let summary = String(reflecting: text) | ||
| guard summary.count > 96 else { return summary } | ||
| return "\(summary.prefix(96))..." | ||
| } | ||
|
|
||
| static func dataSummary(_ data: Data) -> String { | ||
| let prefix = data.prefix(32) | ||
| let prefixData = Data(prefix) | ||
| let hex = prefix.map { String(format: "%02X", $0) }.joined(separator: " ") | ||
| let utf8 = String(data: prefixData, encoding: .utf8) ?? "<non-utf8>" | ||
| let suffix = data.count > prefix.count ? " ..." : "" | ||
| return "len=\(data.count) hex=\(hex)\(suffix) utf8=\(textSummary(utf8))" | ||
| } | ||
| } | ||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| public import CMUXMobileCore | ||
|
|
||
| /// Decides whether a mobile terminal scroll gesture must be sent to the Mac. | ||
| public struct MobileTerminalScrollForwardingPolicy: Sendable { | ||
| /// Creates the forwarding policy. | ||
| public init() {} | ||
|
|
||
| /// Returns whether a scroll should be forwarded to the host surface. | ||
| /// | ||
| /// Primary-screen scrollback is already mirrored into the phone's local | ||
| /// Ghostty surface, so forwarding would make scroll feel network-bound. | ||
| /// Alternate-screen scroll must still reach the host so TUIs with mouse | ||
| /// reporting receive wheel events. | ||
| /// - Parameter activeScreen: The screen currently rendered by the mobile | ||
| /// Ghostty mirror. | ||
| /// - Returns: `true` when the scroll should be sent to the Mac. | ||
| public func shouldForwardToHost( | ||
| activeScreen: MobileTerminalRenderGridFrame.Screen | ||
| ) -> Bool { | ||
| activeScreen == .alternate | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import CMUXMobileCore | ||
| import CmuxMobileTerminalKit | ||
| import Testing | ||
|
|
||
| @Suite struct MobileTerminalScrollForwardingPolicyTests { | ||
| @Test func primaryScreenScrollStaysLocal() { | ||
| let policy = MobileTerminalScrollForwardingPolicy() | ||
|
|
||
| #expect(policy.shouldForwardToHost(activeScreen: .primary) == false) | ||
| } | ||
|
|
||
| @Test func alternateScreenScrollForwardsToHost() { | ||
| let policy = MobileTerminalScrollForwardingPolicy() | ||
|
|
||
| #expect(policy.shouldForwardToHost(activeScreen: .alternate)) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |||||||||||||
| <string>$(CMUX_DEV_TAG)</string> | ||||||||||||||
| <key>CMUXGitSHA</key> | ||||||||||||||
| <string>$(CMUX_GIT_SHA)</string> | ||||||||||||||
| <key>CMUXApiBaseURL</key> | ||||||||||||||
| <string>$(CMUX_API_BASE_URL)</string> | ||||||||||||||
| <key>CFBundleName</key> | ||||||||||||||
| <string>$(PRODUCT_NAME)</string> | ||||||||||||||
| <key>CFBundlePackageType</key> | ||||||||||||||
|
|
@@ -63,8 +65,10 @@ | |||||||||||||
| ONLY WKWebView web content from ATS; the app's own API/auth/pairing | ||||||||||||||
| traffic stays under ATS and still requires HTTPS. NSAllowsLocalNetworking | ||||||||||||||
| alone would not cover routable private-LAN dev servers. --> | ||||||||||||||
| <key>NSAllowsArbitraryLoadsInWebContent</key> | ||||||||||||||
| <true/> | ||||||||||||||
| <key>NSAllowsArbitraryLoads</key> | ||||||||||||||
| <true/> | ||||||||||||||
|
cursor[bot] marked this conversation as resolved.
Comment on lines
+76
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This shared iOS Useful? React with 👍 / 👎. |
||||||||||||||
| <key>NSAllowsArbitraryLoadsInWebContent</key> | ||||||||||||||
| <true/> | ||||||||||||||
|
Comment on lines
+76
to
+79
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Adding
Comment on lines
+76
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove global ATS bypass ( Line 68 enables unrestricted HTTP for all app traffic, which defeats ATS globally and contradicts the scope documented in Lines 62-67 (“ONLY WKWebView”). Keep Suggested fix- <key>NSAllowsArbitraryLoads</key>
- <true/>
<key>NSAllowsArbitraryLoadsInWebContent</key>
<true/>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| </dict> | ||||||||||||||
| <key>UIApplicationSceneManifest</key> | ||||||||||||||
| <dict> | ||||||||||||||
|
|
||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.