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

str::trim_left and str::trim_right method names are misleading #39499

Closed
jimblandy opened this issue Feb 3, 2017 · 7 comments
Closed

str::trim_left and str::trim_right method names are misleading #39499

jimblandy opened this issue Feb 3, 2017 · 7 comments

Comments

@jimblandy
Copy link
Contributor

If you're processing text in a language written from right to left, these methods do the opposite of what they claim. They really refer to the start and end of the slice.

It might be good to have trim_start and trim_end aliases (with _matches variants as well) to help people write clearer code in applications that typically process right-to-left text.

The rest of the API is pretty good about avoiding this mistake; the only other exception I noticed was the std::fmt alignment flags.

@KalitaAlexey
Copy link
Contributor

@jimblandy,
Hi.
I disagree that it is misleading because any slice has the left side (its start) and the right side (its end).
str is a string. It has no direction.

@pthariensflame
Copy link
Contributor

@KalitaAlexey

str is a string. It has no direction.

For that very reason, I agree with @jimblandy’s assessment. Since str has no inherent direction, we shouldn’t use direction-specific words for its API.

@KalitaAlexey
Copy link
Contributor

@jimblandy, @pthariensflame,
Okay. Another problem is that renaming these functions is breaking change.
At maximum a comment may be added.

@pthariensflame
Copy link
Contributor

Well, if the core team felt strongly enough, they could deprecate these names and introduce aliases with better names; they‘ve done exactly that before already.

@jimblandy
Copy link
Contributor Author

jimblandy commented Feb 3, 2017

I should be clear that I have no experience with right-to-left scripts beyond what I've read of the Unicode specification, and what I've picked up from working on Firefox.

I disagree that it is misleading because any slice has the left side (its start) and the right side (its end).

The Unicode standard specifies that Unicode text is always stored in "logical order", meaning the order in which the text would be read. So the Rust string "ערב טוב" (Hebrew for "good evening") actually stores the rightmost character 'ע' first:

assert_eq!("ערב טוב".chars().next(), Some('ע'));

The full details are complicated, but roughly, whitespace characters inherit the direction of the characters around them that have a strong implicit direction. This means that str::trim_left would actually remove whitespace characters that display to the right of that phrase (assuming it's displayed in isolation). That's what I think is confusing. If the function were named trim_start, it would be clear which characters were to be removed regardless of the directionality of the slice's contents.

they could deprecate these names and introduce aliases with better names

Yeah, that's the solution that makes sense to me.

@alexcrichton
Copy link
Member

I think this may be a dupe of #30459?

@jimblandy
Copy link
Contributor Author

Yes, it is. Thanks!

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

No branches or pull requests

4 participants