-
Notifications
You must be signed in to change notification settings - Fork 58
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 StringFilters.camelToSnakeCase filter #24
Changes from 6 commits
74e6c16
7b9be34
b2b6d43
b715499
872c030
fd87056
de6e46b
1089dbe
852a67b
1148a85
b0cdda3
a6f82af
07394dd
cc3ed98
4672b9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,6 +89,44 @@ struct StringFilters { | |
} | ||
} | ||
|
||
/// Converts camelCase to snake_case. Takes an optional Bool argument for making the string lower case, | ||
/// which defaults to true | ||
/// - parameter value: the value to be processed | ||
/// - parameter arguments: the arguments to the function, expecting zero or one argument | ||
/// - returns: the snake case string | ||
/// - throws: FilterError.invalidInputType if the value parameter isn't a string | ||
static func camelToSnakeCase(_ value: Any?, arguments: [Any?]) throws -> Any? { | ||
let toLower = parseBool(from: arguments, index: 0) ?? true | ||
guard let string = value as? String else { throw FilterError.invalidInputType } | ||
|
||
let snakeCase = try snakecase(string) | ||
if toLower { | ||
return snakeCase.lowercased() | ||
} | ||
return snakeCase | ||
} | ||
|
||
/// Parses filter arguments for a boolean value, where true can by any one of: "true", "yes", "1", and | ||
/// false can be any one of: "false", "no", "0". | ||
/// - parameter arguments: an array of argument values, may be empty | ||
/// - parameter index: the index in the arguments array | ||
/// - returns: true or false if a value was parsed, or nil if it wasn't able to | ||
static func parseBool(from arguments: [Any?], index: Int) -> Bool? { | ||
guard index + 1 <= arguments.count else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return nil | ||
} | ||
|
||
let boolArg = (arguments[index] as? String ?? "").lowercased() | ||
switch boolArg { | ||
case "false", "no", "0": | ||
return false | ||
case "true", "yes", "1": | ||
return true | ||
default: | ||
return nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the default here be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. I'll refactor. If it's an optional argument (which I tested that Stencil allows for it), it should return nil, otherwise it should throw. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! |
||
} | ||
} | ||
|
||
/** | ||
This returns the string with its first parameter uppercased. | ||
- note: This is quite similar to `capitalise` except that this filter doesn't lowercase | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,122 @@ class StringFiltersTests: XCTestCase { | |
} | ||
} | ||
|
||
func testParseBool_WithTrueString() throws { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be better to move all these parse tests to a separate XCTestCase. |
||
let value = try StringFilters.parseBool(from: ["true"], index: 0) | ||
XCTAssertTrue(value!) | ||
} | ||
|
||
func testParseBool_WithFalseString() throws { | ||
let value = try StringFilters.parseBool(from: ["false"], index: 0) | ||
XCTAssertFalse(value!) | ||
} | ||
|
||
func testParseBool_WithYesString() throws { | ||
let value = try StringFilters.parseBool(from: ["yes"], index: 0) | ||
XCTAssertTrue(value!) | ||
} | ||
|
||
func testParseBool_WithNoString() throws { | ||
let value = try StringFilters.parseBool(from: ["no"], index: 0) | ||
XCTAssertFalse(value!) | ||
} | ||
|
||
func testParseBool_WithOneString() throws { | ||
let value = try StringFilters.parseBool(from: ["1"], index: 0) | ||
XCTAssertTrue(value!) | ||
} | ||
|
||
func testParseBool_WithZeroString() throws { | ||
let value = try StringFilters.parseBool(from: ["0"], index: 0) | ||
XCTAssertFalse(value!) | ||
} | ||
|
||
func testParseBool_WithInt() throws { | ||
let value = try StringFilters.parseBool(from: [1], index: 0) | ||
XCTAssertNil(value) | ||
} | ||
|
||
func testParseBool_WithDouble() throws { | ||
let value = try StringFilters.parseBool(from: [1.0], index: 0) | ||
XCTAssertNil(value) | ||
} | ||
|
||
func testParseBool_WithEmptyString() throws { | ||
let value = try StringFilters.parseBool(from: [""], index: 0) | ||
XCTAssertNil(value) | ||
} | ||
|
||
func testParseBool_WithEmptyArray() throws { | ||
let value = try StringFilters.parseBool(from: [], index: 0) | ||
XCTAssertNil(value) | ||
} | ||
|
||
func testCamelToSnakeCase_NoArguments() { | ||
let expectations = [ | ||
"string": "string", | ||
"String": "string", | ||
"strIng": "str_ing", | ||
"strING": "str_ing", | ||
"X": "x", | ||
"x": "x", | ||
"SomeCapString": "some_cap_string", | ||
"someCapString": "some_cap_string", | ||
"string_with_words": "string_with_words", | ||
"String_with_words": "string_with_words", | ||
"String_With_Words": "string_with_words", | ||
"String_With_WoRds": "string_with_wo_rds", | ||
"STRing_with_words": "st_ring_with_words", | ||
"string_wiTH_WOrds": "string_wi_th_w_ords", | ||
"": "", | ||
"URLChooser": "url_chooser", | ||
"UrlChooser": "url_chooser", | ||
"a__b__c": "a__b__c", | ||
"__y_z!": "__y_z!", | ||
"PLEASESTOPSCREAMING": "pleasestopscreaming", | ||
"PLEASESTOPSCREAMING!": "pleasestopscreaming!", | ||
"PLEASE_STOP_SCREAMING": "please_stop_screaming", | ||
"PLEASE_STOP_SCREAMING!": "please_stop_screaming!" | ||
] | ||
|
||
for (input, expected) in expectations { | ||
let trueArgResult = try! StringFilters.camelToSnakeCase(input, arguments: ["true"]) as? String | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above, it might be better to separate the boolean parsing logic into it's own method, that can then be tested separately. |
||
XCTAssertEqual(trueArgResult, expected) | ||
} | ||
} | ||
|
||
func testCamelToSnakeCase_OneArgumentAsFalse() { | ||
let expectations = [ | ||
"string": "string", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little nitpicking, but could you ensure the indentation you use is consistent with the rest of the project? Thx |
||
"String": "String", | ||
"strIng": "str_Ing", | ||
"strING": "str_ING", | ||
"X": "X", | ||
"x": "x", | ||
"SomeCapString": "Some_Cap_String", | ||
"someCapString": "some_Cap_String", | ||
"string_with_words": "string_with_words", | ||
"String_with_words": "String_with_words", | ||
"String_With_Words": "String_With_Words", | ||
"String_With_WoRds": "String_With_Wo_Rds", | ||
"STRing_with_words": "ST_Ring_with_words", | ||
"string_wiTH_WOrds": "string_wi_TH_W_Ords", | ||
"": "", | ||
"URLChooser": "URL_Chooser", | ||
"UrlChooser": "Url_Chooser", | ||
"a__b__c": "a__b__c", | ||
"__y_z!": "__y_z!", | ||
"PLEASESTOPSCREAMING": "PLEASESTOPSCREAMING", | ||
"PLEASESTOPSCREAMING!": "PLEASESTOPSCREAMING!", | ||
"PLEASE_STOP_SCREAMING": "PLEASE_STOP_SCREAMING", | ||
"PLEASE_STOP_SCREAMING!": "PLEASE_STOP_SCREAMING!" | ||
] | ||
|
||
for (input, expected) in expectations { | ||
let falseArgResult = try! StringFilters.camelToSnakeCase(input, arguments: ["false"]) as? String | ||
XCTAssertEqual(falseArgResult, expected) | ||
} | ||
} | ||
|
||
func testEscapeReservedKeywords() throws { | ||
let expectations = [ | ||
"self": "`self`", | ||
|
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's already a
lowercase
filter provided by Stencil, so you could already dofoo|camelToSnakeCase|lowercase
instead offoo|camelToSnakeCase(true)
.I think it would be better to keep the lowercasing separate, i.e. let the caller filter thru
|lowercase
in the template, instead of making that logic in code (and with the risk of repeating lowercasing logic in Swift code).Besides, it would allow you to simplify this whole PR by not needing this
parseBool
anymore (I'm surprised thatStencil
doesn't provide a way to parse a boolean argument the same way for all filters already btw?).And I think it would be more understandable to be explicit. Sure you'll lose the "automatic lowercase by default" behavior, that would be unfortunate, but the code would be much simpler (and the risk of regression and inconsistency in the bool argument parsing logic lowered too)
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 might need the
parseBool
logic for other filters we want to refactor (snakeToCamelCaseNoPrefix), but it might then be better to leave that for another PR.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.
Currently
foo|camelToSnakeCase|lowerCase
andfoo|camelToSnakeCase
are equivalent, and I expect that 99% of people will want the default lowercase behavior. You would only need to add a parameter if you wanted snake case with first upper characters, which IMHO will probably be pretty rare. That's why I'd rather keep it as is.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.
Fair enough.