-
Notifications
You must be signed in to change notification settings - Fork 724
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
Added support the existential any
to StoryboardType
and CVarArg
for Swift 5.6 or later.
#1056
base: develop
Are you sure you want to change the base?
Conversation
This pull request is for (Thanks @kntkymt! https://twitter.com/kntkymt/status/1645785310377426944) |
any
to StoryboardType
for Swift 5.6 or laterany
to StoryboardType
and CVarArg
for Swift 5.6 or later.
I found @uhooi's tweet (https://twitter.com/the_uhooi/status/1666836221459595264) and added existential |
@diogot @djbe @fortmarek @marcelofabri @phatblat |
Plz review and release :) |
Close to one year since an accepted PR to the develop branch. Is SwiftGen to be considered dead? :( |
@chipp @diogot @djbe @fortmarek @Liquidsoul @marcelofabri @phatblat Would you please run the checks? Or could you please grant me those permissions? |
I will check later what needs to be done to fix Danger. |
@AliSoftware Danger needs a bot user, can you create it for the organization, please? |
I have made a release of And I pushed my fork on SwiftGenPlugin as well: https://github.com/treastrain/SwiftGenPlugin/tree/support-existential-any This allows us to use it via the Swift Package Manager plugin as follows: // swift-tools-version: 5.6
import PackageDescription
let package = Package(
// ...
dependencies: [
// ...
.package(url: "https://github.com/treastrain/SwiftGenPlugin", branch: "support-existential-any"),
],
targets: [
// ...
.executableTarget(
// ...
plugins: [
.plugin(name: "SwiftGenPlugin", package: "SwiftGenPlugin"),
]
),
],
// ...
) |
Those of you with the appropriate authority can quickly review this pull request and make any workarounds I have in place unnecessary by making a new release. |
Before we start our move away from SwiftGen due to it obviously being abandoned we've implemented the following in our projects build phase script to mitigate this issue: sed -i '' 's/args: CVarArg/args: any CVarArg/g' Strings+Generated.swift It will replace This won't cover all use cases of SwiftGen, but it helps with our :) |
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.
These changes look good to me. I have just a question about some large scope of the #if
that may be smaller to reduce the duplicated code in the stencil?
I've updated some env variables that should hopefully fix danger but I could not relaunch the jobs to check that 🤷
As a sidenote, all this work is for all the embedded stencils all this can be fixed by providing your own stencils without doing some ugly sed
command in your script while enforcing the format of the generating code in your project.
Thank you all for your work and your patience, I will try to do the best I can to address some blocking issues in the following weeks.
#if swift(>=5.6) | ||
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | ||
{% if param.lookupFunction %} | ||
let format = {{ param.lookupFunction }}(key, table, value) | ||
{% else %} | ||
let format = {{param.bundle|default:"BundleToken.bundle"}}.localizedString(forKey: key, value: value, table: table) | ||
{% endif %} | ||
return String(format: format, locale: Locale.current, arguments: args) | ||
} | ||
#else |
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 it not creating a bit too much duplicated code to maintain here?
Cannot we just limit the #if
to the func
line?
Something more like this?
#if swift(>=5.6) | |
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | |
{% if param.lookupFunction %} | |
let format = {{ param.lookupFunction }}(key, table, value) | |
{% else %} | |
let format = {{param.bundle|default:"BundleToken.bundle"}}.localizedString(forKey: key, value: value, table: table) | |
{% endif %} | |
return String(format: format, locale: Locale.current, arguments: args) | |
} | |
#else | |
#if swift(>=5.6) | |
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | |
#else | |
private static func tr(_ table: String, _ key: String, _ args: CVarArg..., fallback value: String) -> String { | |
#endif |
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.
#if swift(>=5.6) | ||
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | ||
{% if param.lookupFunction %} | ||
let format = {{ param.lookupFunction }}(key, table, value) | ||
{% else %} | ||
let format = {{param.bundle|default:"BundleToken.bundle"}}.localizedString(forKey: key, value: value, table: table) | ||
{% endif %} | ||
return String(format: format, locale: Locale.current, arguments: args) | ||
} | ||
#else |
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.
Same #if
scope question here.
#if swift(>=5.6) | |
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | |
{% if param.lookupFunction %} | |
let format = {{ param.lookupFunction }}(key, table, value) | |
{% else %} | |
let format = {{param.bundle|default:"BundleToken.bundle"}}.localizedString(forKey: key, value: value, table: table) | |
{% endif %} | |
return String(format: format, locale: Locale.current, arguments: args) | |
} | |
#else | |
#if swift(>=5.6) | |
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | |
#else | |
private static func tr(_ table: String, _ key: String, _ args: CVarArg..., fallback val | |
#endif |
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.
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.
@Liquidsoul I apologize that my reply is very late. Thank you for reviewing!
#if swift(>=5.6) | ||
private static func tr(_ table: String, _ key: String, _ args: any CVarArg..., fallback value: String) -> String { | ||
{% if param.lookupFunction %} | ||
let format = {{ param.lookupFunction }}(key, table, value) | ||
{% else %} | ||
let format = {{param.bundle|default:"BundleToken.bundle"}}.localizedString(forKey: key, value: value, table: table) | ||
{% endif %} | ||
return String(format: format, locale: Locale.current, arguments: args) | ||
} | ||
#else |
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.
develop
branch (gitflow)develop
as the "base" branch for this Pull Request I'm about to createCHANGELOG.md
file to explain my changes and credit myself(or added
#trivial
to the PR title if I think it doesn't warrant a mention in the CHANGELOG)Hello, I propose to give
StoryboardType
andCVarArg
the SE-0335 "Introduce existential any" implemented since Swift 5.6.This avoids the problem with SE-0362 "Piecemeal adoption of upcoming language improvements" implemented since Swift 5.8, where code generated by SwiftGen causes build errors when SE-0335 upcoming feature flag is enabled.