-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Backported Case2. Left 5 tests failing. #46
Backported Case2. Left 5 tests failing. #46
Conversation
lib/recase/cases/generic.ex
Outdated
list | ||
|
||
:symbol -> | ||
[all, down, up] = Enum.map([32..127, ?a..?z, ?A..?Z], &Enum.to_list/1) |
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.
This treats digits as delimiters as well; maybe it should eliminate digits as well as letters from the list of delimiters.
@am-kantox thanks for this PR! Now, straight to the point: 2,3,5: I guess am just mixing slugs and cases here. It is not really a point of this library to remove any special chars from the strings. Except the explicit separators. If you need to strip these chars, use slugs. So, I agree with you on this one. And I will update docs on making slugs. 4: I guess it is a bug, not sure why this is happening. |
I have fixed a path. Also, I made an explicit set of symbols to be treated as delimiters (confirming your testcase.) |
I am unsure what’s wrong with |
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.
Awesome work @am-kantox! And by "awesome" I mean really awesome.
|
||
This module should not be used directly. | ||
|
||
**NB!** at the moment has no stop words: titleizes everything |
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.
What are "stop words" in this context? We need to document that. Or remove if it does not matter.
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.
Eh. There is a link to Wiki https://en.wikipedia.org/wiki/Letter_case#Title_case saying “fruits and vegetables” should be titleized as “Fruits and Vegetables.”
We do not support and remaining all lowercase. That said, we’d better off removing both warning and link to Wiki.
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.
Sound fine, let's do it.
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.
Not in this PR I guess.
@am-kantox thanks again! I highly appreciate your work. |
I believe failing tests are very opinionated.
1: I doubt I follow why
"-foo-bar"
→"foo/bar"
but"/foo-bar"
→"/foo/bar"
. It’s plain inconsistent (easy to fix though.)2, 3, 5: If we treat
#
as a delimiter, it should go away here and there. Use config option to specify delimiters explicitly.4: How last
E
becomes lowercased? Why?