Skip to content

Conversation

@canndrew
Copy link
Contributor

slice_shift_char splits a str into it's leading char and the remainder of the str. Currently, it returns a (Option<char>, &str) such that:

"bar".slice_shift_char() => (Some('b'), "ar")
"ar".slice_shift_char()  => (Some('a'), "r")
"r".slice_shift_char()   => (Some('r'), "")
"".slice_shift_char()    => (None,      "")

This is a little odd. Either a str can be split into both a head and a tail or it cannot. So the return type should be Option<(char, &str)>. With the current behaviour, in the case of the empty string, the str returned is meaningless - it is always the empty string.

This PR changes slice_shift_char so that:

"bar".slice_shift_char() => Some(('b', "ar"))
"ar".slice_shift_char()  => Some(('a', "r"))
"r".slice_shift_char()   => Some(('r', ""))
"".slice_shift_char()    => None

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

@aturon
Copy link
Contributor

aturon commented Nov 17, 2014

@canndrew Looks like a sensible change to me!

Can you please add [breaking-change] to the bottom of the commit message? We use that to help log changes that will break downstream code. (It'd also be good to copy the text of the PR description into the commit message as well, so that people will see the full rationale.)

Otherwise, with that and a rebase, I'd be happy to take this.

`slice_shift_char` splits a `str` into it's leading `char` and the remainder
of the `str`. Currently, it returns a `(Option<char>, &str)` such that:

    "bar".slice_shift_char() => (Some('b'), "ar")
    "ar".slice_shift_char()  => (Some('a'), "r")
    "r".slice_shift_char()   => (Some('r'), "")
    "".slice_shift_char()    => (None,      "")

This is a little odd. Either a `str` can be split into both a head and a
tail or it cannot. So the return type should be `Option<(char, &str)>`.
With the current behaviour, in the case of the empty string, the `str`
returned is meaningless - it is always the empty string.

This commit changes slice_shift_char so that:

    "bar".slice_shift_char() => Some(('b', "ar"))
    "ar".slice_shift_char()  => Some(('a', "r"))
    "r".slice_shift_char()   => Some(('r', ""))
    "".slice_shift_char()    => None

[breaking-change]
@canndrew
Copy link
Contributor Author

All done. Should be building again now.

@bors bors merged commit 197a0ac into rust-lang:master Nov 18, 2014
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.

5 participants