Parser for list of parameters #8

djbe · 2017-01-15T00:04:32Z

Implements these issues:

Allow to inject custom parameters to the Stencil context via command line SwiftGen#253
User should be able to specify additional template parameters on the command line. krzysztofzablocki/Sourcery#14

This parses a list of strings (Array<String>) into a dictionary ([String: Any]). You can use it as follows:

let template = ...load template...
var context = ...generate context...

context["args"] = Parameters.parse(items: items)
let result = try template.render(context)

djbe · 2017-01-28T15:03:22Z

Is this what you guys had in mind? Should I add some sort of method for easily adding this to an (existing) context?

AliSoftware

You should add tests for:

Invalid syntax & invalid key error cases
more than one-level-deep structured keys(foo.bar.baz.qux=1)
inconsistent structures mixing keys used both as root for sub keys and as keys for raw values (foo=1 foo.bar=2 foo.bar=3 foo=4)

djbe · 2017-01-28T15:13:12Z

invalid keys?

AliSoftware · 2017-01-28T15:13:44Z

Sources/Parameters.swift

+		return result
+	}
+
+	private static func parse(item: Parameter, result: StringDict) throws -> StringDict {


Maybe rename that private function to splitAndRepeat or similar? Making it have the same name as the public one was confusing about its intend on first read

AliSoftware · 2017-01-28T15:16:13Z

I meant whatever use case the ParameterError.invalidKey(key: String, value: String) is for.

djbe · 2017-01-28T15:16:46Z

Yeah just saw that. Honestly can't remember when that'd happen. Which means the code needs comments!

AliSoftware · 2017-01-28T15:19:18Z

Honestly can't remember when that'd happen. Which means the code needs comments!

And tests for those cases you thought about when writing that but forgot since then… making those cases likely to be forgotten again in the future and leading to regressions 😜

AliSoftware · 2017-01-28T16:28:41Z

Sources/Parameters.swift

+    var result = StringDict()
+    for parameter in parameters {
+      result = try parse(item: parameter, result: result)
+    }


parameters.reduce(StringDict()) { try Parse(item: $1, résultats: $0) }?

AliSoftware · 2017-01-28T16:30:58Z

Sources/Parameters.swift

+    var result = StringDict()
+    for parameter in parameters {
+      result = try parse(parameter: parameter, result: result)
+    }


parameters.reduce(StringDict()) { try Parse(item: $1, résultats: $0) }?

That french autocorrect 😆

AliSoftware · 2017-01-28T16:32:32Z

Sources/Parameters.swift

+  }
+
+  // a valid key is not empty and only alphanumerical
+  private static func validate(key: String) -> Bool {


Does this means 1 is a valid key too?

Right now, yeah. Shouldn't matter that much, say I want to create a (fake) tuple.

Ok. Fine by be

AliSoftware · 2017-01-28T16:33:32Z

Sources/Parameters.swift

+    var result = StringDict()
+    for parameter in parameters {
+      result = try parse(parameter: parameter, result: result)
+    }


AliSoftware · 2017-01-28T16:35:02Z

Tests/TestSuites/ParametersTests.swift

+    } catch ParametersError.invalidSyntax {
+      // That's the expected exception we want to happen
+    } catch let error {
+      XCTFail("Unexpected error occured while parsing: \(error)")


Isn't there some XCTAssertThrows or similar?

No idea, I'll look into it. The structure is just based on the SwiftGenKit (colors) tests

That doesn't mean I was right at that time 😂

And I'm the testing noob 🤣

AliSoftware · 2017-02-14T15:49:49Z

Sources/Parameters.swift

-      if parts.count == 2 {
-        return (key: parts[0], value: parts[1])
+      if parts.count >= 2 {
+        return (key: parts[0], value: parts.dropFirst().joined(separator: "="))
      } else if let part = parts.first, parts.count == 1 && validate(key: part) {


Does that mean that foo.bar.baz isn't a valid boolean key? (as validate(key: "foo.bar.baz") will return false)?

AliSoftware · 2017-02-15T13:02:51Z

Sources/Parameters.swift

  }
+
+  private static let notAlfanumericsAndDot: CharacterSet = {


s/alfa/alpha 😄

AliSoftware · 2017-02-15T13:04:15Z

Tests/StencilSwiftKitTests/ParametersTests.swift

@@ -86,6 +90,7 @@ class ParametersTests: XCTestCase {
  }

  func testInvalidKey() {
+    // key may only be alfanumeric or '.'


s/alfa/alpha/

djbe force-pushed the feature/parameter-parser branch from 22112a5 to 579ab3a Compare January 16, 2017 17:20

AliSoftware force-pushed the master branch 2 times, most recently from 9625443 to e9e33c6 Compare January 22, 2017 16:09

AliSoftware force-pushed the feature/parameter-parser branch from 579ab3a to f528fe1 Compare January 22, 2017 16:46

AliSoftware added a commit that referenced this pull request Jan 22, 2017

Add CHANGELOG entry for #8

f528fe1

djbe force-pushed the feature/parameter-parser branch from f528fe1 to 245fca3 Compare January 28, 2017 14:56

djbe pushed a commit that referenced this pull request Jan 28, 2017

Add CHANGELOG entry for #8

245fca3

djbe requested a review from krzysztofzablocki January 28, 2017 15:01

djbe requested a review from AliSoftware January 28, 2017 15:03

AliSoftware requested changes Jan 28, 2017

View reviewed changes

AliSoftware reviewed Jan 28, 2017

View reviewed changes

djbe force-pushed the feature/parameter-parser branch from c13cb84 to 7294fac Compare January 28, 2017 16:29

AliSoftware reviewed Jan 28, 2017

View reviewed changes

AliSoftware approved these changes Jan 28, 2017

View reviewed changes

AliSoftware added the status: ready to merge (waiting for CI) label Jan 28, 2017

djbe force-pushed the feature/parameter-parser branch 2 times, most recently from f8ecf4c to ae51ca0 Compare January 30, 2017 17:15

djbe pushed a commit that referenced this pull request Jan 30, 2017

Add CHANGELOG entry for #8

a983f21

djbe modified the milestone: 1.0.0 Jan 31, 2017

djbe and others added 4 commits February 13, 2017 18:51

add parameter parser

f70eff4

Add CHANGELOG entry for #8

c4117bb

better key validation + more extensive tests

7a93338

further test params structure

550282b

djbe added 5 commits February 13, 2017 18:52

renamed recursive method

db74700

reduce

6186196

rewrite tests that expect failure to use XCTAssertThrowsError

406180f

readme entry

c40ed43

remove warning

5159be2

djbe force-pushed the feature/parameter-parser branch from 6aafc52 to 5159be2 Compare February 13, 2017 17:55

djbe mentioned this pull request Feb 14, 2017

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

Merged

djbe added 2 commits February 14, 2017 16:19

Parse parameters without values as booleans with a value of true

dd774e2

update documentation and parse edge case

77a8b27

AliSoftware reviewed Feb 14, 2017

View reviewed changes

djbe added 2 commits February 14, 2017 18:45

handle keys with subkeys but no value (boolean)

91f4cc0

add enrich context function

476a384

krzysztofzablocki approved these changes Feb 15, 2017

View reviewed changes

AliSoftware reviewed Feb 15, 2017

View reviewed changes

Sources/Parameters.swift Outdated

}

private static let notAlfanumericsAndDot: CharacterSet = {

Copy link

Contributor

AliSoftware Feb 15, 2017

Choose a reason for hiding this comment

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

s/alfa/alpha 😄

AliSoftware reviewed Feb 15, 2017

View reviewed changes

s/alfa/alpha

717d7a3

AliSoftware merged commit fe47125 into master Feb 15, 2017

AliSoftware deleted the feature/parameter-parser branch February 15, 2017 15:58

AliSoftware mentioned this pull request Feb 15, 2017

Allow to inject custom parameters to the Stencil context via command line SwiftGen/SwiftGen#253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser for list of parameters #8

Parser for list of parameters #8

djbe commented Jan 15, 2017 •

edited

Loading

djbe commented Jan 28, 2017

AliSoftware left a comment

djbe commented Jan 28, 2017

AliSoftware Jan 28, 2017

AliSoftware commented Jan 28, 2017

djbe commented Jan 28, 2017

AliSoftware commented Jan 28, 2017

AliSoftware Jan 28, 2017

AliSoftware Jan 28, 2017

djbe Jan 28, 2017

AliSoftware Jan 28, 2017

AliSoftware Jan 28, 2017

djbe Jan 28, 2017

AliSoftware Jan 28, 2017

AliSoftware Jan 28, 2017

AliSoftware Jan 28, 2017

djbe Jan 28, 2017

AliSoftware Jan 28, 2017

djbe Jan 28, 2017

AliSoftware Feb 14, 2017

AliSoftware Feb 15, 2017

AliSoftware Feb 15, 2017

Parser for list of parameters #8

Parser for list of parameters #8

Conversation

djbe commented Jan 15, 2017 • edited Loading

djbe commented Jan 28, 2017

AliSoftware left a comment

Choose a reason for hiding this comment

djbe commented Jan 28, 2017

Choose a reason for hiding this comment

AliSoftware commented Jan 28, 2017

djbe commented Jan 28, 2017

AliSoftware commented Jan 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djbe commented Jan 15, 2017 •

edited

Loading