Skip to content
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

Split on Word Boundary Functionality #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kevinwuhoo
Copy link
Contributor

This is a first draft that addresses #1. I've added support to optionally truncate to a word boundary but would like some comments. The support I've actually added is the ability to truncate by a regex. I'm not sure, but this might be overkill for this problem. Another library that does text truncation, clamp.js takes a list of strings to split on, but this implementation with a regex is a superset of that functionality. I've added this to the demo for testing with a few simple options. You can see it live at: https://rawgit.com/kevinwuhoo/trunk8/split-on/demo.html.

The approach it takes is that post truncation, it removes the filled text, scans for the nearest match for the given regex, removes the trailing text, then re-appends the filled text. This approach is not optimal, but it adds this functionality without modification of the logic or surrounding code. I'm also not sure that splitOn is the right name for this feature.

@rviscomi
Copy link
Owner

rviscomi commented Feb 3, 2015

Thanks for working on this. I may take some time to review the code thoroughly, but one thing I noticed in your demo is that truncating in the center doesn't seem to respect the splitOn setting.

Overall, my first impression is to agree that accepting a regular expression may indeed be overkill. I had imagined this being a simple boolean option. Maybe you could wrap this functionality in another option "preserveWholeWords" or something that sets an internal splitOn property to \s. If people wanted, they could expose the inner workings of the splitOn functionality for more control. Honestly, I can't think of a use case other than word boundaries that would be useful. Splitting on vowels is cool for the sake of a programming exercise but I'm not sure the concept has much practical use. I can be convinced otherwise.

@kevinwuhoo
Copy link
Contributor Author

I forgot that I took the SIDES.center case out before I submitted this. I removed it because it would make the middle line abnormally short and made paragraphs look weird. Though, that's not a good enough reason to not be feature complete and introduce unexpected behavior.

I think it could be useful to split on punctuation or specifically periods, commas, etc depending on the use. But yeah, I agree I'm not sure there are many uses beyond single characters. The vowel example was really just a good test to make sure everything worked as expected.

Also, after submitting this PR last night, I found that splitOn interacts poorly with parseHTML sometimes. Will have to look into more and report back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants