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

Implement last for EscapeUnicode #33103

Merged
merged 2 commits into from
May 20, 2016
Merged

Conversation

ranma42
Copy link
Contributor

@ranma42 ranma42 commented Apr 20, 2016

The implementation is quite trivial as the last character is always '{'.
As a side-effect it also improves the implementation of last for EscapeUnicode.

Part of #24214, split from #31049.

Maybe this (and the other changes that I will split from #31049) should wait for a test like ed_iterator_specializations to be added. Would it be sufficient to do the same for each possible escape length?

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

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

@alexcrichton
Copy link
Member

Thanks! Could you add a few tests for this as well?

@nagisa
Copy link
Member

nagisa commented May 3, 2016

Thanks! Could you add a few tests for this as well?

@ranma42 ping, in case you missed the original request by Alex (this happens to me way too often).

@ranma42
Copy link
Contributor Author

ranma42 commented May 4, 2016

Thanks for the heads-up!
I added a test for characters of any legitimate length and for the NUL character.
The test checks the behaviour of nth, last and count in a way that is similar to ed_iterator_specializations, but it also exercises them on partially-used iterators.

@ranma42
Copy link
Contributor Author

ranma42 commented May 19, 2016

@alexcrichton ping-back ;)
I tried to make the test pretty extensive as in my queue I also have patches for the specialisation of nth and count.

@alexcrichton
Copy link
Member

@bors: r+ 8169fa2

Oops, sorry about that!

bors added a commit that referenced this pull request May 19, 2016
Implement `last` for `EscapeUnicode`

The implementation is quite trivial as the last character is always `'{'`.
As a side-effect it also improves the implementation of `last` for `EscapeUnicode`.

Part of #24214, split from #31049.

Maybe this (and the other changes that I will split from #31049) should wait for a test like `ed_iterator_specializations` to be added. Would it be sufficient to do the same for each possible escape length?
@bors
Copy link
Contributor

bors commented May 19, 2016

⌛ Testing commit 8169fa2 with merge 1ec80f6...

@bors bors merged commit 8169fa2 into rust-lang:master May 20, 2016
@ranma42 ranma42 deleted the escape-unicode-last branch May 25, 2016 10:00
Manishearth added a commit to Manishearth/rust that referenced this pull request May 28, 2016
…richton

Implement `count` for `EscapeUnicode`

and cleanup the code for `count` for `EscapeDefault` (instead of repeating the `match` for `size_hint` and `count`).

This PR marks EscapeUnicode and EscapeDefault as ExactSizeIterator. The constraints for the trait implementations held even before this PR, but I am not sure if this is something we want to guarantee/expose (I would love feedback on this, especially on what would be the appropriate way to handle stabilisation, if needed).

Part of rust-lang#24214, split from rust-lang#31049.

The test for `count` was added in rust-lang#33103.
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