kinow commented on pull request #179: URL: https://github.com/apache/commons-text/pull/179#issuecomment-717526516
>The existing method needs a review first IMO. For this patch though, I don't see why applying the function to a string should change its size, that's just weird to me. I think that's just the way it was designed. You provide the delimiters for each part of a camel-cased String. Say you have a snake-cased String `s="no-more-spaces"`, then `CaseUtils.toCamelCase(s, false, '-')` gives you `noMoreSpaces`. The function expects a delimiter that tells it where a new token to be camel-cased starts, and replaces this delimiter when camel-casing. IMHO, it's more intuitive to be consistent with the removal of delimiters for every non-empty String. So `s="a---"` passing through `CaseUtils.toCamelCase(s, false, '-')` gives you `[a]`, and `s="---" gives you `[]`, and not `[___]` (i.e. three empty spaces) as `master` does at the moment. >One problem I see with the current implementation is the use of the default character set in the call to toLowerCase(). At the very least, we need a test to account for the "Turkish Surprise", see https://garygregory.wordpress.com/2015/11/03/java-lowercase-conversion-turkey/ and see also the last comment about “Kedi”.toUpperCase() which I am not sure applies to title case. +1 but I think this doesn't need to block this change, we can create a separate ticket about surrogate pairs in `CaseUtils` methods for camel-case (I suspect if we inspect more of [text], we would find more places where we could find similar issues). They can be fixed separately. I'd be fine with either way. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
