Deprecated variables #13
Conversation
djbe
force-pushed
the
feature/deprecated-variables
branch
from
c36d16e
to
08313a5
Compare
Sources/Stencil/ColorsContext.swift
Outdated
|
|
||
| return [ | ||
| "colors": colorMap, | ||
| "param": ["enumName": enumName], |
There was a problem hiding this comment.
The way SwiftGen's CLI inject command line --param x=y parameters means that this will be overridden by extra params, right?
There was a problem hiding this comment.
It does. I'm going to have to modify the param parser to merge context vars.
|
Ok I'm gonna trust you'll not forget to fix StencilSwiftKit to merge the params keys in the context other that that I think it's ready to merge… except if you want to merge the circle ci PR first (and rebase on top of it afterwards) to have a greener CI status on which we'd need more confident? 😉 |
|
Yeah the circle prs go first.
…Sent from my iPhone
On 28 Feb 2017, at 01:18, AliSoftware ***@***.***> wrote:
Ok I'm gonna trust you'll not forget to fix StencilSwiftKit to merge the params keys in the context other that that I think it's ready to merge… except if you want to merge the circle ci PR first (and rebase on top of it afterwards) to have a greener CI status on which we'd need more confident? 😉
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
djbe
force-pushed
the
feature/deprecated-variables
branch
from
04f244d
to
00865b3
Compare
|
@AliSoftware After SwiftGen/StencilSwiftKit#29 is merged, I can adapt this and SwiftGen/templates#28 if needed, and we can finally get this merged. The wiki page should be up to date, but I'll do another check to be sure. |
Sources/Stencil/StringsContext.swift
Outdated
| if entry.types.count > 0 { | ||
| let params: [String: Any] = [ | ||
| "types": entry.types.map { $0.rawValue }, | ||
| result["parameters"] = entry.types.map { $0.rawValue } |
There was a problem hiding this comment.
I wonder if we shouldn't name this entry types or placeholderTypes instead of parameters. After all, that's more the list of types than a list of parameters. We call if parameters because we assume it's gonna be used to derive the parameters of an enum with associated values, but maybe not…
| print("Error: \(error.localizedDescription)") | ||
| } | ||
|
|
||
| let result = parser.stencilContext(sceneEnumName: "XCTStoryboardsScene", segueEnumName: "XCTStoryboardsSegue") |
There was a problem hiding this comment.
Shouldn't we use this PR to migrate stencilContext(sceneEnumName:, segueEnumName: ) to stencilContext(params: […]) instead, since the --sceneEnumName and --segueEnumName flags of swiftgen are also deprecated in favour of --param?
There was a problem hiding this comment.
I didn't think the parsers/generators would use parameters from CLI, or will they?
There was a problem hiding this comment.
For template stuff, such as enumName, these will be stored in params and don't affect SwiftGenKit. Should we have flags that change the generator's behaviour, I think we should have those as actual CLI documented flags/options (via Commander).
There was a problem hiding this comment.
Mmmh maybe I misunderstood, where is that stencilContext(sceneEnumName: segueEnumName: ) method defined, is it just a helper method for the tests here?
CHANGELOG.md
Outdated
| [David Jennes](https://github.com/djbe) | ||
| [#13](https://github.com/SwiftGen/SwiftGenKit/issues/13) | ||
| * For each string, the `params` variable and it's subvariables (such as `names`, `count`, ...) have been replaced by `parameters`, which is an array of types. | ||
| * The `strings`, `structuredStrings` and `tableName` have been replaced by `tables`, which is an array of string tables, each with a `name` and a `strings` property. |
There was a problem hiding this comment.
Nitpicking but I find it more readable / logical to inverse the order of the first 2 bullet points. First talk top level, then talk about structure for each string.
|
Don't forget to update the comments (which summarise the structure of each context) as well as the documentation in |
|
Sure, but quite a few of those (code comments) are really old (see images), so I'm nog going to bother documenting deprecated variables. |
|
Indeed, not worth documenting deprecate variables (and forget again to update the comments when we remove them in 5.0 😄), but it's a while since they haven't been in sync so I figured it was a good occasion to do so |
djbe
force-pushed
the
feature/deprecated-variables
branch
from
cae5286
to
ecbf047
Compare
|
Documentation has been updated (also in other repo's). I'm merging this now, we can always tweak things before release if needed. |
Implements #5
Works together with SwiftGen/templates#28.