path: Refactor set_extension and contains_nul #13395
Conversation
|
Could you elaborate a little more on what the use case is for requiring the Container trait? I would expect that it's just "one more trait" to implement, and it doesn't have a particularly compelling reason to require. |
|
It was more that it felt implied, but I guess you're right and it could be restrictive/bothersome to require it. Would you instead accept if I instead extend |
|
I would figure that I think that this trait may be seeing a bit more prime time than it was intended for, sadly, it's mostly just to make |
|
Updated commits do as mentioned above, feel free to close pr if unwanted. |
|
Would you be ok with omitting the addition of the |
| Some(ref s) => { | ||
| if contains_nul(s) { None } | ||
| else { | ||
| Some(unsafe { GenericPathUnsafe::new_unchecked(*s) }) | ||
| } |
There was a problem hiding this comment.
Stylistically, this if/else should look like:
if contains_nul(s) {
None
} else {
Some(...)
}
|
Nuked byte_len() addition and fixed |
alexcrichton
changed the title
…xcrichton Also some minor cleanup in Path related to this.
…nishearth Add lint for unnecessary lifetime bounded &str return Closes rust-lang#305. Currently implemented with a pretty strong limitation that it can only see the most basic implicit return, but this should be fixable by something with more time and brain energy than me. Cavets from rust-lang#13388 apply, as I have not had a review on my clippy lints yet so am pretty new to this. ``` changelog: [`unnecessary_literal_bound`]: Add lint for unnecessary lifetime bounded &str return. ```
Also some minor cleanup in Path related to this.