PuerkitoBio / goquery

A little like that j-thing, only in Go.
BSD 3-Clause "New" or "Revised" License
13.9k stars 917 forks source link

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

Closed amikai closed 1 day ago

amikai commented 1 month ago

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

mna commented 1 month ago

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

amikai commented 1 month ago

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

mna commented 1 month ago

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!