Skip to content

Conversation

@sfackler
Copy link
Member

My primary use case here is sending strings across the wire where the
intermediate storage is a byte array. The new method ends up avoiding a
copy.

@huonw
Copy link
Contributor

huonw commented Aug 24, 2013

I think the naming convention is into for things that convert without allocation like this, so into_bytes.

@sfackler
Copy link
Member Author

@huonw fixed.

@alexcrichton
Copy link
Member

that become unmergeable quickly :(

My primary use case here is sending strings across the wire where the
intermediate storage is a byte array. The new method ends up avoiding a
copy.
@sfackler
Copy link
Member Author

@alexcrichton rebased

thestinger added a commit that referenced this pull request Aug 24, 2013
@thestinger thestinger merged commit e311a1e into rust-lang:master Aug 24, 2013
@sfackler sfackler deleted the str-to-bytes branch August 31, 2013 00:05
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 11, 2024
…plitting-at-newlines, r=Jarcho

Issue 8733: Suggest `str.lines` when splitting at hard-coded newlines

Fixes rust-lang#8733.
```
changelog: [`splitting_strings_at_newlines`]: New lint that suggests `str.lines` over splitting at hard-coded newlines
```

This is my first PR to Clippy and one of my first Rust PRs in general -- please feel free to nitpick, I'm thankful for any opportunity to learn! I'd be especially interested in feedback to the following points:

* Is checking for `'\n'`, `"\n"`, and `"\r\n"` as arguments to `split` enough, or should we do more (e.g. checking for constants that have those values if that is possible)?
* Could the code be written in a more idiomatic way?
* Is the default `".."` for `snippet` a good choice? I copied it from other uses of `snippet` in the code base, but I'm not entirely sure.
* Is the category `suspicious` a good choice?
* Is the suggestion applicability `MaybeIncorrect` a good choice? I used it because the return type of `lines` is not exactly the same as that of `split`.
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