-
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 1 commit
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 |
---|---|---|
|
@@ -8,7 +8,7 @@ | |
<BuildActionEntries> | ||
<BuildActionEntry | ||
buildForTesting = "YES" | ||
buildForRunning = "NO" | ||
buildForRunning = "YES" | ||
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. Is there a reason this was changed? I don't think it does much. 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. Ah, probably AppCode made the change since I was running unit tests. Will revert. 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. Seems to still be there? 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. Will fix, I must have checked it in inadvertently again |
||
buildForProfiling = "NO" | ||
buildForArchiving = "NO" | ||
buildForAnalyzing = "NO"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,39 @@ class StringFiltersTests: XCTestCase { | |
} | ||
} | ||
|
||
func testCamelToSnakeCase() { | ||
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 result = try! StringFilters.camelToSnakeCase(input) as? String | ||
XCTAssertEqual(result, expected) | ||
} | ||
} | ||
|
||
func testEscapeReservedKeywords() { | ||
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.
Is this to avoid
URLChooser
-->URL_Chooser
?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.
According to https://en.wikipedia.org/wiki/Snake_case,
Hello_world
is valid output. No idea if that's what we want though.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.
Hmm, don't think I've ever seen snake case with caps. The scenario I'm trying to get to is auto-generating json_field_names from camelCase, which tend to be all lower case.
Should I break it out into two different ones? camelToSnakeCase and camelToLowerSnakeCase? Or does Stencil already provides a way to combine something like this: {{ "someTextHere"|camelToSnakeCase|lowercase }}?
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.
I haven't seen it either, I was just wondering what the correct / default behaviour should be. Looking at some libraries in other languages, the default seems to be lowercase, so let's do that 👍
You could always add a parameter to the filter to disable the lowercasing (see for example the stencil join filter http://stencil.fuller.li/en/latest/builtins.html#built-in-filters).
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.
Unfortunately the Wikipedia page doesn't give any definitive rules for upper snake case. For instance, as the StringFilter.snakecase() function is currently written, URLChooser becomes URL_Chooser, not Url_Chooser. I couldn't really find anything that said that'd be right or wrong, so I think for now I'll just stick with that implementation. The filter itself will take one optional boolean parameter for lowercasing, defaults to true.