goquery icon indicating copy to clipboard operation
goquery copied to clipboard

Question: Is it possible support the iterator in version 1.23?

Open amikai opened this issue 1 year ago • 3 comments

Golang 1.23 will support the iter package and define the types iter.Seq and iter.Seq2 as iterators. My idea is that maybe Each can return an iterator. Then the user can use the for loop on it easily.

func (s *Selection) EachIter() iter.Seq2[int, *Selection]

At client side:

for i, s := range doc.Find(".left-content article .post-title").Each() {
    title := s.Find("a").Text()
    fmt.Printf("Review %d: %s\n", i, title)
}

Reference

https://tip.golang.org/doc/go1.23 https://pkg.go.dev/iter@master

amikai avatar Jul 19 '24 17:07 amikai

Hello,

Thanks for raising that very interesting idea! I haven't played with the new function-based iterators yet, but from what you mention and a quick glance at the iter package, that looks doable and would make a lot of sense to me.

I think having a new Selection method called EachIter as you have in your first example would be a good name, to avoid breaking compatibility and keep supporting the existing Each and EachWithBreak, though both could be documented as deprecated.

Only thing is that this would make go1.23 required for goquery, but that's fine as that would be part of a new release, so older releases would still be available for those that cannot use go1.23 yet.

If you (or anyone else reading this) are interested in experimenting with this and proposing a PR, I'd be happy to review and assist in getting this merged! (if you do, please make sure to add tests for the new method :pray: )

Thanks, Martin

mna avatar Jul 19 '24 19:07 mna

Hi @mna I think Each, EachWithBreak don't need to deprecated, because EachIter do not chain the Selection itself. See the implementation in https://github.com/PuerkitoBio/goquery/pull/483

amikai avatar Jul 20 '24 09:07 amikai

I think Each, EachWithBreak don't need to deprecated, because EachIter do not chain the Selection itself

good point, you’re right! Thanks for the PR, I’ll take a look sometime next week!

mna avatar Jul 20 '24 21:07 mna