Identifiers starting with numbers when combining `swiftIdentifier` and `snakeToCamelCase`

[dDomovoj]

Generated output example: ...enum UniversLTStd: String, FontConvertible { case 65Bold = "UniversLTStd-Bold" case 67BoldCondensed = "UniversLTStd-BoldCn" case 57Condensed = "UniversLTStd-Cn" case 47LightCondensed = "UniversLTStd-LightCn" case 47LightCondensedOblique = "UniversLTStd-LightCnObl" case 59UltraCondensed = "UniversLTStd-UltraCn" }...

[djbe]

This is a duplicate of https://github.com/SwiftGen/SwiftGen/issues/289. While it is the right place for this issue, most of the discussion already happened there.

[djbe]

I'm reopening this, as it's still not fixed, and we should discuss what the best solution would be.

[djbe]

Given an input of "25 Ultra Light" and depending on the order, we get:

• swiftIdentifier|snakeToCamelCase:"true" -> "25UltraLight"
• swiftIdentifier|snakeToCamelCase:"false" -> "_25UltraLight"
• snakeToCamelCase:"true"|swiftIdentifier -> "_25_Ultra_Light"
• snakeToCamelCase:"false"|swiftIdentifier -> "_25_Ultra_Light"

[djbe]

Should we just use the 2nd case (don't strip leading underscores)? Or modify the function to accept an enum:

• none (default): don't strip leading _
• all: strip all leading _
• keepOne: strip all but one leading _

I think we'll probably use the keepOne everywhere with maybe 1 or 2 exceptions.

[AliSoftware]

From a logical standpoint, I think we should apply swiftIdentifier last, because we want to apply filters to make it look nice / the way we prefer first, and end the list of filters with "now that we have what we hoped, make just sure it's gonna be valid in the end". While applying swiftIdentifier first then adding more transforms after that doesn't guarantee that the final result after all the transforms are still gonna be valid Swift.

What I don't understand is why, once transformed "25 Ultra Light" to CamelCase using snakeToCamelCase, we don't end up with 25UltraLight — which should at last be transformed into valid swift identifier _25UltraLight at the end. Should we improve the snakeToCamelCase filter?

[AliSoftware]

Side note: we should add some examples containing spaces in that filter's documentation to make it clear of its behaviour with spaces. I think that's actually the core of the issue (and imho snakeToCameCase should just remove the spaces and capitalise the letter after said space)

[djbe]

Not sure about that though, we'd lose information if the name was "25 ultra light".

[djbe]

And, swiftIdentifier uppercases the first letter --> we still need to apply lowerFirstWord

[AliSoftware]

Not sure about that though, we'd lose information if the name was "25 ultra light".

How so? "25 ultra light"|snakeToCamelCase should still result in "25UltraLight". Which would then result in "_25UltraLight" once we apply swiftIdentifier to that result.

[djbe]

snakeCaseToCamelCase only thinks in terms of underscores, it doesn't (and shouldn't) care about other characters.

      if try containsAnyLowercasedChar(string) {
        let comps = string.components(separatedBy: "_")
        unprefixed = comps.map { titlecase($0) }.joined(separator: "")
      }

[AliSoftware]

Currently, the code does this which doesn't take spaces into account. So "25 Ultra Light" is unchanged under snakeToCamelCase. That's the main problem, as imho it should replace spaces with underscores first. Or we should apply a filter that does that first.

[AliSoftware]

Ok, so let's apply {{font.name|replace:" ","_"|snakeToCamelCase|swiftIdentifier|lowerFirstWord then.

[djbe]

Well, that's why we have swiftidentifier.

Maybe we should swiftIdentifier|snakeToCamelCase:true|swiftIdentifier.

[djbe]

I woulnd't go with the replace filter, it's too limited for future swift changes. What do we do with other invalid characters?

[AliSoftware]

No, swiftIdentifier does more than replacing spaces with underscores. It also replaces other invalid swift characters. Applying the filter twice seems odd to me logically…

Let's take more convoluted examples to look at what could be logically right

What about "12 @ 34 % 56 + 78 Hello world", I think we want to get "_12_34_56_78_Hello_world" as a result? Because spaces should not be turned to underscores by snakeToCameCalse? Or maybe we would prefer "_123456___78_Hello_world"? If so, for "25 Ultra Light" we would want "_25_UltraLight" to be consistent (invalid chars kept as "")?

Or would we want "12 @ 34 % 56 + 78 Hello world" to result to "_12345678HelloWorld" ? Or to "_12_34_56_78HelloWorld" (but what would be the rule of removing underscores generated from invalid chars vs one from spaces vs ones that would avoid numbers next to each others be glued together?)

[djbe]

Right, no idea what's the ideal solution. personally as a end result I'd expect "_25UltraLight". For those other examples, not a clue.

Template writer shouldn't have to worry about all these edge cases, I'm starting to think the swiftIdentifier (or another) filter should do the whole thing? Even the lowerFirstWord part. For swift2 we can just add upperFirstLetter where needed.

Edit: such a filter could even accept an (optional) swift version.

[AliSoftware]

swiftIdentifier shouldn't lower the first word.

• because swiftIdentifier|lowerFirstWord|upperFirstLetter isn't identical to swiftIdentifier, e.g. URLChooser would become UrlChooser in Swift 2
• Because there are a lot of places where swift identifiers don't require lowercase first letter, e.g. a type name. What if we want to write a template which generates one class or enum type per font family, that type name should keep the uppercase first letter as it would be used as a type name

It's definitely not the role of swiftIdentifier to do the lowerFirstLetter. But for the other parts, especially avoiding to always chain all those other filters in all the places we use them in the templates, I agree that a convenience filter with options would be nicer to use, even if in the end it's only just a convenience filter applying multiple existing filters in sequence.

Maybe the rule should be: (1) replace " [a-z]" (space followed by lowercase letter) with "[A-Z]" (remove space and uppercase first letter), then just apply the current version of swiftIdentifier on the result.

• That way 25 ultra light will first become 25UltraLight then swiftIdentifier will transform it to _25UltraLight
• And 12 @ 34 % 56 + 78 Hello world will first become 12 @ 34 % 56 + 78HelloWorld then _12___34___56___78HelloWorld, which would be acceptable.
• And 12_34 hey ya 5 will first become 12_34HeyYa 5 then be transformed by swiftIdentifier into _12_34HeyYa_5 which seems ok to me too.

So in practice, it could make sense to make snakeToCamelCase also (optionally, we can add more parameters for that filter if needed) not only replace "" but also replace " " with { "" + uppercase next letter }, and only if the next letter is "capitalizable" (so numbers or symbols which are unchanged when capitalized should keep the "" or " " before them instead of remove them?)

[djbe]

item 1: we also have a lowerFirstLetter now 😄

Should that convenience filter also apply the snakeToCamelCase filter? Something like what you had before, then: replace:" ","_"|snakeToCamelCase|swiftIdentifier

We could add this as a variation of the swiftIdentifier behind a parameter. If true, prettify the string, otherwise (default) only the current behaviour.

[AliSoftware]

The more I think about it, the more I think making the swiftIdentifier accepting a parameter like :none/:normal/:false (default) vs :pretty would make sense.

Making that parameter a string describing the "mode" (e.g. pretty) will allow us to provide multiple prettification / convention variants in the (near?) future. Like one that just removes spaces and uppercase the letter after the removed space, but keep original underscores unchanged, another which removes all spaces AND underscores and uppercase the letter after, another which could have even more complex rules in the future to deal differently with existing underscores vs spaces vs special chars, etc.

The first variant that we'd want to implement will probably be equivalent to the current replace:" ","_"|snakeToCamelCase|swiftIdentifier indeed (so like you suggested, just maybe use "true" as a parameter but some other string like "pretty" or "all" or whatnot.

---

NB: In any case, I don't think it would be the job of the swiftIdentifier filter, whatever its prettification variant, to lowercase the first letter. That should still be left for the template writer to call lowerFirstWord or lowerFirstLetter or upperFirstLetter or whatever they need depending on the context (swift version + type or instance name + other stuff, like maybe for strings people want the string portion unchanged so they can copy/paste keys from their Localizable.strings and use them as is, etc)