Tracking Issue for `Iterator::intersperse` · Issue #79524 · rust-lang/rust · Git...

m-ou-se commented on Jan 13

#80567 is adding intersperse_with to this feature.

cyqsimon commented on Mar 28 •

I would like to suggest a simpler, alternative name for this method - weave. At the time of comment, the word weave is not yet used in Rust std.

Intersperse is quite verbose (especially when compared to Vec::join), but more importantly rarely used in common English. weave is much more common and concise in comparison.

ibraheemdev commented on Mar 29 •

Scala and haskell both use intersperse. Scalaz calls it intercalate...

tmplt commented on May 3

I would like to suggest a simpler, alternative name for this method - weave.

weave is not on the list of terms I would search for when looking for the expected behavior. I'd even consider it vanity.

Intersperse is quite verbose (especially when compared to Vec::join), but more importantly rarely used in common English. weave is much more common and concise in comparison.

A verbose function name describes its effect better. More so if the term is uncommon; there is then less ambiguity. intersperse does exactly what is says: intersperse elements with a common separator.

why not use join and join_with for iterators too?

fosskers commented on May 18

Scala and haskell both use intersperse. Scalaz calls it intercalate...

intercalate is slightly different.

intersperse :: a -> [a] -> [a] 

intercalate :: [a] -> [[a]] -> [a] 

kaylynn234 commented on Jun 20 •

why not use join and join_with for iterators too?

I feel like join implies the creation of something monolithic from a sequence and a separator, i.e

let items = vec!["foo", "bar", "blah"];
let result = items.join(" ");

will create the string foo bar blah, and I don't have any issue with that.

But in my mind intersperse better communicates the idea of placing separators in between a sequence, without creating something monolithic using the results of that.

I suppose I mean that if I intersperse an iterator of &strs with " ", it makes sense to me that I get an iterator back.
But if I have an iterator of &strs, and the method is named join, I feel like it's a little less clear. Without looking further into it, it might seem like that I'd get a concatenated String back, or similar.
For other types it may be less unclear but I'm still unsure on whether calling the method join is a good fit.

arnabanimesh commented on Jun 20 •

I feel like join implies the creation of something monolithic from a sequence and a separator

That's true, but we are using vec::join or slice::join and that respects the pattern matching and it is not monolithic. Then vec::join should be changed too. If Rust already acknowledges that join be non monolithic, then I suggest we stick with the same ideology.

But if I have an iterator of &strs, and the method is named join, I feel like it's a little less clear. Without looking further into it, it might seem like that I'd get a concatenated String back, or similar.

I'm always collecting to String after intersperse.

My personal preference aside, vec::join creates a vec if the initial data type is not &str. So it is just intersperse which happens to create a String in a specific scenario.

Personally, if it were not as verbose I would have gone for intersperse. If join was not used in vec or slice, I would not root for it. But weave doesn't sound good IMO.

My main point is that let's not have two different terms do the same thing based on datatype.

pickfire commented on Jun 30 •

Personally, if it were not as verbose I would have gone for intersperse. If join was not used in vec or slice, I would not root for it. But weave doesn't sound good IMO.

I did join for Iterator in the past #75738 which accepts the current Join that does both but it wasn't accepted because it requires allocation or Iterator is in std so it couldn't be implemented, splitting it as two functions works but may not be that discoverable.

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 String or &String easily with &str and such? I think cases more than that we need some intervention which the API will not be ergonomic. I think maybe it only works well with &'static str.

In the current example, I believe it's too basic and limited, requires &'static str and copied.

let hello = ["Hello", "World", "!"].iter().copied().intersperse(" ").collect::<String>();

I think we need to think more about the ergonomics of the API before stabilization.

Shouldn't there also be something like flat_intersperse? (intersperse but with an array (impl IntoIterator) as the argument)

overlisted commented 29 days ago •

Continuing the flat_intersperse discussion myself, here's one way how it can be implemented (TODO make that Vec a generic list):

enum Step {
    PutNext,
    PutSeparator(usize),
}

pub struct FlatIntersperse<I: Iterator>
where
    I::Item: Clone,
{
    separator: Vec<I::Item>,
    iter: Peekable<I>,
    step: Step,
}

impl<I: Iterator> FlatIntersperse<I>
where
    I::Item: Clone,
{
    pub fn new(iter: I, separator: Vec<I::Item>) -> Self {
        Self {
            iter: iter.peekable(),
            separator,
            step: Step::PutNext,
        }
    }
}

impl<I> Iterator for FlatIntersperse<I>
where
    I: Iterator,
    I::Item: Clone,
{
    type Item = I::Item;

    fn next(&mut self) -> Option<Self::Item> {
        if self.iter.peek().is_none() {
            None
        } else {
            match self.step {
                Step::PutNext => {
                    self.step = Step::PutSeparator(0);

                    self.iter.next()
                }
                Step::PutSeparator(i) => {
                    let next_i = i + 1;

                    self.step = if next_i == self.separator.len() {
                        Step::PutNext
                    } else {
                        Step::PutSeparator(next_i)
                    };

                    Some(self.separator[i].clone())
                }
            }
        }
    }
}

m-ou-se commented 12 days ago

@rfcbot merge

rfcbot commented 12 days ago •

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.

rfcbot commented 7 days ago

This is now entering its final comment period, as per the review above.

SoniEx2 commented 7 days ago

could call it "sep" for separator?