-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Tracking Issue for str::{floor, ceil}_char_boundary #93743
Comments
Have methods that return |
So, that would work for purely code-based splitting, but if a user were to perform more-complicated splitting by something like graphemes or words, they'd want to be able to look at the string both before and after the truncation point, not just at that point. Since some cases required the index specifically, I decided to keep the API surface small. But adding extra variants that directly slice the string could be very useful too, like the ones you offered. |
I don't see why choice to panic instead of return |
The main reason I chose that was that floor_char_boundary can never panic, and I wanted parity for the API with ceil_char_boundary. Do you have a case that would benefit from such an API? |
One can panic the other not is not parity either. I advice to avoid panic when possible, while there is already a number of method that panic instead of return an Option or a Result, adding new item should probably follow better guide line, specially since Rust could be use in Kernel linux, having method that panic on primitive type would automatically ban them from use. On this case I wonder if Or something else that remove the panic. |
So, the reason why one can panic and one can't is basically because of the fact that indexes can't go below zero, and so you can always round down to 0, which is always a valid char boundary. Here, the context is that if you want to limit the size of a string in bytes, you can "round down" from that to a valid index, such that you can slice up to that. For ceil... the general idea, if I'm being honest, is to round "up" the recommended number of bytes. So, for example, if you want to limit to 1000 bytes, but are fine allowing up to 1004 to include the last character. I guess in this case, it would be okay to return len instead of panicking, but I don't know it this would make sense in all cases. Definitely open to suggestions on what you think is best, but IMHO the flooring version should definitely never panic, and always return a valid index. |
I think that removal of panic from this function is a must, the usage is to correctly find the bounds of a char and prevent panicking when doing string processing, it seems not good that a function to prevent that can indeed panic. |
So, I was poking around the uses of and am thinking that maybe the solution to a non-panicking version of Effectively, if you pass an index past the end of the string, it's treated as the end of the string, and rounds "up" to the end of the string. |
I believe there is some confusion here. The "(split) index" is not lost during
That is, it's strictly more information than the original Also because |
…boundary, r=m-ou-se Don't panic in ceil_char_boundary Implementing the alternative mentioned in this comment: rust-lang#93743 (comment) Since `floor_char_boundary` will always work (rounding down to the length of the string is possible), it feels best for `ceil_char_boundary` to not panic either. However, the semantics of "rounding up" past the length of the string aren't very great, which is why the method originally panicked in these cases. Taking into account how people are using this method, it feels best to simply return the end of the string in these cases, so that the result is still a valid char boundary.
… r=m-ou-se Don't panic in ceil_char_boundary Implementing the alternative mentioned in this comment: rust-lang/rust#93743 (comment) Since `floor_char_boundary` will always work (rounding down to the length of the string is possible), it feels best for `ceil_char_boundary` to not panic either. However, the semantics of "rounding up" past the length of the string aren't very great, which is why the method originally panicked in these cases. Taking into account how people are using this method, it feels best to simply return the end of the string in these cases, so that the result is still a valid char boundary.
… r=m-ou-se Don't panic in ceil_char_boundary Implementing the alternative mentioned in this comment: rust-lang/rust#93743 (comment) Since `floor_char_boundary` will always work (rounding down to the length of the string is possible), it feels best for `ceil_char_boundary` to not panic either. However, the semantics of "rounding up" past the length of the string aren't very great, which is why the method originally panicked in these cases. Taking into account how people are using this method, it feels best to simply return the end of the string in these cases, so that the result is still a valid char boundary.
… r=m-ou-se Don't panic in ceil_char_boundary Implementing the alternative mentioned in this comment: rust-lang/rust#93743 (comment) Since `floor_char_boundary` will always work (rounding down to the length of the string is possible), it feels best for `ceil_char_boundary` to not panic either. However, the semantics of "rounding up" past the length of the string aren't very great, which is why the method originally panicked in these cases. Taking into account how people are using this method, it feels best to simply return the end of the string in these cases, so that the result is still a valid char boundary.
… r=m-ou-se Don't panic in ceil_char_boundary Implementing the alternative mentioned in this comment: rust-lang/rust#93743 (comment) Since `floor_char_boundary` will always work (rounding down to the length of the string is possible), it feels best for `ceil_char_boundary` to not panic either. However, the semantics of "rounding up" past the length of the string aren't very great, which is why the method originally panicked in these cases. Taking into account how people are using this method, it feels best to simply return the end of the string in these cases, so that the result is still a valid char boundary.
Feature gate:
#![feature(round_char_boundary)]
This is a tracking issue for
str::{floor, ceil}_char_boundary
.Public API
Steps / History
Unresolved Questions
ceil_char_boundary
panic when indices are out of bounds? Don't panic in ceil_char_boundary #112387 is an existing PR to change this to return the length instead.The text was updated successfully, but these errors were encountered: