-
Notifications
You must be signed in to change notification settings - Fork 216
SwiftUI greeter #529
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
SwiftUI greeter #529
Conversation
swift/Ice/SwiftUI-greeter/README.md
Outdated
| @@ -0,0 +1,17 @@ | |||
| # SwiftUI Greeter | |||
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 find the name SwiftUI-greeter inconsistent with the other names. What about a simpler GreeterUI?
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.
What about GreeterApp?
| import Ice | ||
| import SwiftUI | ||
|
|
||
| class GreeterClient: ObservableObject { |
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.
Would be nice to add some comments.
| import Ice | ||
| import SwiftUI | ||
|
|
||
| @main |
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.
Missing comments.
| /// @return The greeting. | ||
| string greet(string name); | ||
| } | ||
| } No newline at end of file |
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.
missing newline
|
|
||
| import SwiftUI | ||
|
|
||
| struct ContentView: View { |
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.
Missing comment
swift/Ice/SwiftUI-greeter/.gitignore
Outdated
| @@ -0,0 +1,2 @@ | |||
| # Exclude per-user settings | |||
| project.xcworkspace/xcuserdata/ No newline at end of file | |||
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.
File should end in newline.
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.
Can we exclude this file so that it always picks up the latest nightly?
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.
need to try if Xcode complains when it is missing.
| } | ||
| } | ||
| #if os(macOS) | ||
| .formStyle(.grouped) // or .columns (macOS 14+) |
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.
We only support macOS 15+, might as well use the new features.
| Text("Host:") | ||
| TextField("e.g. localhost", text: $host) |
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.
Not sure that we need the extra Text field. Maybe just TextField("Host:", ... and TextField("Name:",...
| Text("Host:") | |
| TextField("e.g. localhost", text: $host) | |
| Text("Host:") | |
| TextField("e.g. localhost", text: $host) |
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.
Is that common on iOS, the TextField first argument only shows up when empty doesn't it? I added the label to make clear what the input is about, I'm fine dropping it...
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'm not sure to be honest.
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.
Pull Request Overview
This PR introduces a SwiftUI-based greeter application that uses Ice to send greeting requests. The updates include new documentation in the README, implementation of core functionality in GreeterView, GreeterClient, and GreeterApp, along with necessary project configuration and asset updates.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| swift/Ice/GreeterApp/README.md | Added introductory documentation |
| swift/Ice/GreeterApp/GreeterView.swift | Implemented the main UI view for user input and greeting display |
| swift/Ice/GreeterApp/GreeterClient.swift | Added client functionalities to handle greeting requests via Ice |
| swift/Ice/GreeterApp/GreeterApp.swift | Configured the main application entry point using environment injection |
| swift/Ice/GreeterApp/GreeterApp.entitlements | Defined App Sandbox and network permissions |
| swift/Ice/GreeterApp/Greeter.ice | Added Slice definition for the Greeter interface |
| swift/Ice/GreeterApp/Assets.xcassets/* | Configured asset catalogs and app icon assets |
| swift/Ice/GreeterApp/.gitignore | Added ignore configuration for personal settings |
| swift/Ice/GreeterApp/GreeterApp.xcodeproj/project.pbxproj | Updated project configuration and build phases |
Files not reviewed (1)
- swift/Ice/GreeterApp/GreeterApp.xcodeproj/project.xcworkspace/contents.xcworkspacedata: Language not supported
Comments suppressed due to low confidence (1)
swift/Ice/GreeterApp/GreeterClient.swift:16
- Consider handling initialization failures more gracefully instead of using fatalError. A user-friendly error handling approach may improve application resiliency in production.
fatalError("Failed to initialize communicator: \(error)")
| import Ice | ||
| import SwiftUI | ||
|
|
||
| /// Manages the Ice communicator and provides a method to send a greeting request to the Greeter service. |
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.
greet request
A SwiftUI greeter client application