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

Document LTR vs RTL wrt trim_* #31202

Merged
merged 2 commits into from
Feb 2, 2016
Merged

Document LTR vs RTL wrt trim_* #31202

merged 2 commits into from
Feb 2, 2016

Conversation

steveklabnik
Copy link
Member

Fixes #30459

Fun fact: i wanted to write "Arabic" and "Hebrew" in Arabic and Hebrew, but vim kept doing the copy/paste in the wrong direction.

@rust-highfive
Copy link
Collaborator

r? @gankro

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Perhaps LTR and RTL could be expanded? Took me a second to figure out what that meant. Or it may be canonical enough and I could just be ignorant.

r=me either way!

@huonw
Copy link
Member

huonw commented Jan 26, 2016

I'm not sure this should close #30459: on one hand, it's as good as we can do wrt that issue for now, but, on the other, it'll be easier to lose track of it once we do get the tooling if the issue is closed. Thoughts?

@steveklabnik
Copy link
Member Author

Or it may be canonical enough and I could just be ignorant.

I'm not sure either... they're canonical for people who talk about text, but do we want to assume that? I'm not sure. @SimonSapin what do you think?

I'm not sure this should close #30459:

Ah, that's true. I guess I was seeing it through the docs lens, but you're probably right.

@SimonSapin
Copy link
Contributor

I agree that not every reader of the docs is necessarily familiar with these acronyms, and not using them is easy here. “LTR vs RTL” could be replaced with “Text directionality” for example.

@SimonSapin
Copy link
Contributor

And yes, not everyone is familiar with text directionality either, but it’s less obscure than acronyms and explained below.

@steveklabnik
Copy link
Member Author

@bors: r=alexcrichton rollup

@nodakai
Copy link
Contributor

nodakai commented Jan 30, 2016

Can you add several doctests? For example, according to Wikipedia, a Hebrew word for Hebrew (language) is "עברית" and its romanization is "ivrit". Its last character is "ת" aka "tav" which corresponds to a roman alphabet "t".

fn f(s: &str, fc: char, lc: char) {
    assert!(fc == s.trim_left().chars().next().unwrap());
    assert!(lc == s.trim_right().chars().last().unwrap());
}

fn main() {
    f("  English  ", 'E', 'h');
    f("  עברית  ",
'ע',
'ת');
}

@steveklabnik
Copy link
Member Author

re- r? @alexcrichton

I've added the doc tests that @nodakai asked about 👍

@rust-highfive rust-highfive assigned alexcrichton and unassigned Gankra Feb 1, 2016
@alexcrichton
Copy link
Member

@bors: r+ 743bad4

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Feb 1, 2016
Fixes rust-lang#30459

Fun fact: i wanted to write "Arabic" and "Hebrew" in Arabic and Hebrew, but vim kept doing the copy/paste in the wrong direction.
@steveklabnik
Copy link
Member Author

@bors: r=alexcrichton rollup

I did a dumb thing, and it's fixed now.

@bors
Copy link
Contributor

bors commented Feb 2, 2016

📌 Commit c0ace5d has been approved by alexcrichton

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Feb 2, 2016
Fixes rust-lang#30459

Fun fact: i wanted to write "Arabic" and "Hebrew" in Arabic and Hebrew, but vim kept doing the copy/paste in the wrong direction.
bors added a commit that referenced this pull request Feb 2, 2016
@bors bors merged commit c0ace5d into rust-lang:master Feb 2, 2016
@steveklabnik steveklabnik deleted the gh30459 branch June 19, 2016 20:30
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.

8 participants