-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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 Iterator::intersperse
#79524
Comments
#80567 is adding |
Add Iterator::intersperse_with This is a follow-up to rust-lang#79479, tracking in rust-lang#79524, as discussed rust-lang#79479 (comment). ~~Note that I had to manually implement `Clone` and `Debug` because `derive` insists on placing a `Clone`-bound on the struct-definition, which is too narrow. There is a long-standing issue # for this somewhere around here :-)~~ Also, note that I refactored the guts of `Intersperse` into private functions and re-used them in `IntersperseWith`, so I also went light on duplicating all the tests. If this is suitable to be merged, the tracking issue should be updated, since it only mentions `intersperse`. Happy New Year! r? `@m-ou-se`
Add Iterator::intersperse_with This is a follow-up to rust-lang#79479, tracking in rust-lang#79524, as discussed rust-lang#79479 (comment). ~~Note that I had to manually implement `Clone` and `Debug` because `derive` insists on placing a `Clone`-bound on the struct-definition, which is too narrow. There is a long-standing issue # for this somewhere around here :-)~~ Also, note that I refactored the guts of `Intersperse` into private functions and re-used them in `IntersperseWith`, so I also went light on duplicating all the tests. If this is suitable to be merged, the tracking issue should be updated, since it only mentions `intersperse`. Happy New Year! r? ``@m-ou-se``
Expand docs on Iterator::intersperse Unstable feature in rust-lang#79524. This expands on the docs to bring them more in line with how other methods of `Iterator` are demonstrated.
…=joshtriplett Implement Extend and FromIterator for OsString Add the following trait impls: - `impl Extend<OsString> for OsString` - `impl<'a> Extend<&'a OsStr> for OsString` - `impl FromIterator<OsString> for OsString` - `impl<'a> FromIterator<&'a OsStr> for OsString` Because `OsString` is a platform string with no particular semantics, concatenating them together seems acceptable. I came across a use case for these trait impls in artichoke/artichoke#1089: Artichoke is a Ruby interpreter. Its CLI accepts multiple `-e` switches for executing inline Ruby code, like: ```console $ cargo -q run --bin artichoke -- -e '2.times {' -e 'puts "foo: #{__LINE__}"' -e '}' foo: 2 foo: 2 ``` I use `clap` for command line argument parsing, which collects these `-e` commands into a `Vec<OsString>`. To pass these commands to the interpreter for `Eval`, I need to join them together. Combining these impls with `Iterator::intersperse` rust-lang#79524 would enable me to build a single bit of Ruby code. Currently, I'm doing something like: ```rust let mut commands = commands.into_iter(); let mut buf = if let Some(command) = commands.next() { command } else { return Ok(Ok(())); }; for command in commands { buf.push("\n"); buf.push(command); } ``` If there's interest, I'd also like to add impls for `Cow<'a, OsStr>`, which would avoid allocating the `"\n"` `OsString` in the concatenate + intersperse use case.
I would like to suggest a simpler, alternative name for this method -
|
Scala and haskell both use |
A verbose function name describes its effect better. More so if the term is uncommon; there is then less ambiguity. |
why not use |
intersperse :: a -> [a] -> [a]
intercalate :: [a] -> [[a]] -> [a] |
I feel like let items = vec!["foo", "bar", "blah"];
let result = items.join(" "); will create the string But in my mind I suppose I mean that if I intersperse an iterator of |
That's true, but we are using
I'm always collecting to My personal preference aside, Personally, if it were not as verbose I would have gone for My main point is that let's not have two different terms do the same thing based on datatype. |
I did In a sample code in helix, let res = sel
.ranges
.into_iter()
.map(|range| format!("{}/{}", range.anchor, range.head))
.collect::<Vec<String>>()
.join(","); How to join In the current example, I believe it's too basic and limited, requires let hello = ["Hello", "World", "!"].iter().copied().intersperse(" ").collect::<String>(); I think we need to think more about the ergonomics of the API before stabilization. |
@rfcbot merge |
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
could call it "sep" for separator? |
I expected this code to compile
Instead, I need to create such boilerplate because I need to have exactly the same input type
Or what should I do in similar situations? |
What is the status of this feature? |
Thank you @Fishrock123
Leaving aside the above initial plan described in #88967 (comment) and taking into consideration that #89151 appears to be stalled, this feature will likely take some years to be stabilized, if ever. |
Yes, it looks like someone interested would need to take up working on #89151 |
This has been open for years. The nice solution of changing trait resolution had no progress for years either, and may be blocked by other refactorings too. Could we reconsider renaming the method?
How about Or |
My personal bikeshed-report: I'd suggest |
I think |
We could also consider calling it |
If someone can make a decision, I'm willing to change the Stone Of Shame around my neck for the Stone Of Triumph and submit a PR. |
I agree that |
It can't be Both join functions call their argument a separator. Therefore I think |
I see, that makes sense. It does seem like |
My personal favorite would be
The only purpose of having a |
@camelid I was arguing for not using
|
I think |
@workingjubilee my disagreement with wording like |
I think
|
I love a good bikeshedding just as everyone, yet before this Tracking Issue is overwhelmed with it, I suggest to wait and see on the merit of #79524 (comment) in it's own right. My guess is that the PR to simply evade blockers now being on I-libs-api-nominated will yield some insights on that. |
I just encountered a situation where this method would have been an excellent solution to a problem I was facing, but I was restricted to stable Rust and couldn't use another crate. It's a shame that it's been marked as unstable for so long given that it only seems to be blocked via a naming conflict with itertools. Renaming seems like a reasonable solution to me, but I would also note that the corresponding depreciation request on the itertools github seemed to point to there not being a full understanding on their part that this was not being marked as stable because of the naming conflict. They wanted to wait for stability to deprecate their own method, leading to the current state of things. |
better to have
Examples: I specifically ran into this while looking for solution for KZG encoding that requrires a tranfroming a byte stream into 32 byte symbol with the first (big edian) byte to be zero. https://docs.eigenlayer.xyz/eigenda/rollup-guides/blob-encoding As it stands , this API looks like a "alternate" implementation of zip() with infinite iterator over the second iterator. |
:( looks like a direct transation of this Would be gereat to know how to solve this usecase |
To give an update, we're currently trying to work around the naming conflict with a language-level feature: #89151 (comment) |
This is a tracking issue for the
Iterator::intersperse
andintersperse_with
methods. It is based onitertools::intersperse
and is the iterator equivalent ofVec::join
.The feature gate for the issue is
#![feature(iter_intersperse)]
.Public API
Steps / History
intersperse
: AddIterator::intersperse
#79479intersperse_with
: Add Iterator::intersperse_with #80567Iterator::intersperse()
#88548(UPDATE: stabilization was reverted because there was a lot of breakage: Stabilizing Iterator::intersperse breaks a large number of crates #88967)
impl<I: FusedIterator> FusedIterator for Intersperse<I>
? (Can be added after stabilization, so not urgent.)Unresolved Questions
None.
The text was updated successfully, but these errors were encountered: