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

Utility functions #1129

Closed
wants to merge 8 commits into from
Closed

Utility functions #1129

wants to merge 8 commits into from

Conversation

Yoric
Copy link
Contributor

@Yoric Yoric commented Nov 4, 2011

A few utility functions that I use in my implementation of ropes.
I will send a separate pull request for ropes once I have finished cleaning up the code.

@Yoric
Copy link
Contributor Author

Yoric commented Nov 4, 2011

Ah, well, I have finally decided to add the library to the pull request.

@Yoric
Copy link
Contributor Author

Yoric commented Nov 4, 2011

I should add that this library is my first real foray into Rust. I fully expect numerous efficiencies, unexpected memory copies, etc. Please do not hesitate to point me to any error I may have committed.

@Yoric
Copy link
Contributor Author

Yoric commented Nov 4, 2011

Actually, there is a difference between loop_chars and iter_chars: the second cannot be interrupted.

@brson
Copy link
Contributor

brson commented Nov 5, 2011

So this looks really great! Thank you. I made a number of comments, but mostly on style. One thing to note is that functions without naturaldocs comments (starting with "Function:") will not show up in the library documentation at all, so every public function needs documentation.

This patch does establish a naming convention 'loop' and a protocol (return bool) for iterators that can break early, which I guess is as good as any.

I would very much like it if you provided tests for std::rope. They would live in src/test/stdtest/rope.rs

@Yoric
Copy link
Contributor Author

Yoric commented Nov 5, 2011

Normally, everything is done, except for the explicit export in rope.rs, which I still need to ponder.

@brson
Copy link
Contributor

brson commented Nov 5, 2011

Thanks, Yoric. Integrated.

@brson brson closed this Nov 5, 2011
coastalwhite pushed a commit to coastalwhite/rust that referenced this pull request Aug 5, 2023
celinval pushed a commit to celinval/rust-dev that referenced this pull request Jun 4, 2024
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.

3 participants