Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

ignore module for import, if it matches the current module #19

Merged
merged 3 commits into from
Feb 16, 2017

Conversation

djbe
Copy link
Member

@djbe djbe commented Feb 14, 2017

This simply introduces the use of the env and param variables, and uses it to fix SwiftGen/SwiftGen#254. The templates work as-is, but for the fix to work, SwiftGen/StencilSwiftKit#8 must first be merged and added to the contexts.

Replacing (and removing) enumName and other variables is left for another PR.

@djbe
Copy link
Member Author

djbe commented Feb 15, 2017

How to test this though...

Couple of options:

  • create extra context plists for variations with env and param variables (one for each + one with both). Disadvantage: contexts that can't be auto generated from running StencilSwiftKit based script.
  • modify the testing method to call enrich(context:parameters:) with variations. Disadvantage: how do we name the output files for each variation?

@AliSoftware
Copy link
Collaborator

Wondering if we could unit-test that change to ensure it properly ignores the module for both cases (env and param)?

@djbe
Copy link
Member Author

djbe commented Feb 15, 2017

I've tested them locally, and they work.
I'm currently trying to write something that fits the current testing model to test the variations.

@AliSoftware
Copy link
Collaborator

Should I merge now and you'll write unit tests later when you figure out something, or should I wait?

@djbe
Copy link
Member Author

djbe commented Feb 15, 2017

Gimme a couple of minutes and I'll push something, we can discuss it then

Generate variations of a context.

- Parameter name: The name of the context
- Parameter contex: The context itself
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/contex/context/

@djbe djbe force-pushed the feature/use-env-and-params branch from 734bc40 to b7983d1 Compare February 15, 2017 18:05
contextVariations: StoryboardsiOSTests.variations)
}

private static func enrich(_ context: [String: Any], env: [String: Any]? = nil, param: [String: Any]? = nil) -> [String: Any] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if the default value for env shouldn't be ProcessInfo().environment instead of nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno, it is a "stub" method, and we have no control over environment variables.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, yeah. I confused this method with the one from StencilSwiftKit (which we should maybe use instead to ensure consistency?)

Copy link
Member Author

@djbe djbe Feb 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would we control the env. variables then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either by modifying the method in StencilSwiftKit to pass the env as parameters, with default value ProcessInfo().environment so that except from tests we don't have to specify it… or by stubbing ProcessInfo via swizzling :P

Copy link
Collaborator

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for SwiftGen/StencilSwiftKit#19 to be merged to remove the enrich test helper function and use the one from StencilSwiftKit instead

@djbe djbe force-pushed the feature/use-env-and-params branch from b7983d1 to ff03e90 Compare February 15, 2017 23:38
@AliSoftware AliSoftware added this to the 1.0.0 milestone Feb 15, 2017
@djbe djbe force-pushed the feature/use-env-and-params branch from ff03e90 to 78a34bb Compare February 16, 2017 09:12
Copy link
Collaborator

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpickings, otherwise OK to merge

]
```

For easier use, you can use the `enrich(context:parameters:)` function to add the following variables to a context:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StencilContext.enrich(context:parameters:)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, that's in fact from the StencilSwiftKit pod, I missed it there ^^

@@ -134,7 +134,7 @@ extension XCTestCase {
- Parameter contextVariations: Optional closure to generate context variations.
*/
func test(template templateName: String, contextNames: [String], outputPrefix: String, directory: Fixtures.Directory, contextVariations: VariationGenerator? = nil) {
let template = SwiftTemplate(templateString: Fixtures.template(for: "\(templateName).stencil"),
let template = StencilSwiftTemplate(templateString: Fixtures.template(for: "\(templateName).stencil"),
environment: stencilSwiftEnvironment())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation

@djbe djbe force-pushed the feature/use-env-and-params branch from 78a34bb to 3286da5 Compare February 16, 2017 10:54
@djbe djbe force-pushed the feature/use-env-and-params branch from 3286da5 to 498d415 Compare February 16, 2017 11:00
@djbe djbe merged commit dfdd6c3 into master Feb 16, 2017
@djbe djbe deleted the feature/use-env-and-params branch February 16, 2017 14:26
@FelixLisczyk
Copy link

Hi @djbe,

I've noticed that you didn't include this change in the storyboards-osx-swift3 template. Can you add it here or should I submit a new PR?

@djbe
Copy link
Member Author

djbe commented Feb 22, 2017

Oh wow that's unfortunate 😦 I'll add it real quick!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude module from storyboard file's imports
3 participants