-
-
Notifications
You must be signed in to change notification settings - Fork 58
Bring back the terminal backend to life. #210
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?
Conversation
It renames 'CursesBackend' to TermKit, as now TermKit has various console drivers, and curses is just one of them. The challenge is that TermKit deals with screen sizes like 80x24, not things like 600x400, so both the default sizes and spacing in the codebase will need to compensate for that. The AppBackend introduces a defaultStackSpacingAmount which is the spacing that VStack and HStack would use, it is currently kept as a default to the existing value, but for the TermKit backend, this value is set to zero. Now HStack and VStack rather than storing a resolved value like 'spacing: Int', they store the request 'spacing: Int?', which is resolved before it is actually needed by querying the backend for this data. There is also a new 'limitScreenBounds' that is a no-op for most, but on the curses cases, ensures that a misuse of the defaultSize does not trickle up and down the stack with unrealistic view sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had a quick look out of interest and left a few comments (mostly formatting related). I'll change this PR to a draft for now
// name: "CursesBackend", | ||
// dependencies: ["SwiftCrossUI", "TermKit"] | ||
// ), | ||
.target( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation (and for the TermKit package above and TermKitBackend product above that)
// ), | ||
.target( | ||
name: "TermKitBackend", | ||
dependencies: ["SwiftCrossUI", .product(name: "TermKit", package: "TermKit")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use "TermKit"
spelling instead of .product(name: "TermKit", package: "TermKit")
todo() | ||
} | ||
|
||
public func limitScreenBounds(_ bounds: SIMD2<Int>) -> SIMD2<Int> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should be reworked to func clampWindowSize(_ size: SIMD2<Int>, for window: Window) -> SIMD2<Int>
. Renamed for clarity, and extra parameter added for future proofing (future purposes could want the window in question to access properties such as scaling factors).
proposedWindowSize: isFirstUpdate && isProgramaticallyResizable | ||
? (newScene ?? scene).defaultSize | ||
: backend.size(ofWindow: window), | ||
? backend.limitScreenBounds((newScene ?? scene).defaultSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indent these two hanging lines
} | ||
|
||
public func runInMainThread(action: @escaping @MainActor () -> Void) { | ||
#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS) || os(visionOS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indent this body two levels
It renames 'CursesBackend' to TermKit, as now TermKit has various console drivers, and curses is just one of them.
The challenge is that TermKit deals with screen sizes like 80x24, not things like 600x400, so both the default sizes and spacing in the codebase will need to compensate for that.
The AppBackend introduces a defaultStackSpacingAmount which is the spacing that VStack and HStack would use, it is currently kept as a default to the existing value, but for the TermKit backend, this value is set to zero.
Now HStack and VStack rather than storing a resolved value like 'spacing: Int', they store the request 'spacing: Int?', which is resolved before it is actually needed by querying the backend for this data.
There is also a new 'limitScreenBounds' that is a no-op for most, but on the curses cases, ensures that a misuse of the defaultSize does not trickle up and down the stack with unrealistic view sizes.