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 graphemes and grapheme_indices into two methods #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KamilaBorowska
Copy link

s.extended_graphemes() is more readable than s.graphemes(true), as you don't have to think what does true mean here. Those methods were implemented as default methods in order to preserve backward compatibility if somebody implemented UnicodeSegmentation for their own types.

This is a feature, so second number in semantic version should be increased.

s.extended_graphemes() is more readable than s.graphemes(true), as
you don't have to think what does true mean here. Those methods were
implemented as default methods in order to preserve backward
compatibility if somebody implemented UnicodeSegmentation for their
own types.
@Manishearth
Copy link
Member

i kinda think making it extended_graphemes() will make people use it less. (perhaps make the false case legacy_graphemes?)

@tapeinosyne
Copy link

@Manishearth
i kinda think making it extended_graphemes() will make people use it less. (perhaps make the false case legacy_graphemes?)

A preference for legacy_graphemes is apparent in #22 as well.

@KamilaBorowska
Copy link
Author

KamilaBorowska commented Oct 14, 2017

Oh, there was already an issue about this in issue tracker. Somehow missed that.

Unfortunately it's not really possible to have optional parameters in Rust, so changing graphemes(true) into graphemes() would be a breaking change. Definitely a good choice for unicode-segmentation 2.0.0 however.

But now that I think about it... I think it's feasible to have a trait with a different name which exports graphemes() method. This could be however confusing and UnicodeSegmentation is a good name, so it's probably not a good choice.

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.

None yet

3 participants