goquery icon indicating copy to clipboard operation
goquery copied to clipboard

Support IterEach based on 1.23 Iterator

Open amikai opened this issue 1 year ago • 3 comments

Resolve #482

What's change

  • Bump to golang 1.23
  • Support IterEach

Note: Considering that Go 1.23 is not stable yet, I am not sure if merging that PR would be risky.

Detail

Test cases: TestEachIter is similar with TestEach TestEachIterWithBreak is similar withTestEachWithBreak

Benchmarks: BenchmarkEachIter is similar with BenchmarkEach BenchmarkEachIterWithBreak is similar with BenchmarkEachWithBreak

amikai avatar Jul 20 '24 03:07 amikai

Hi @mna,

I have reorganized the code to address compatibility issues. This means we don't need to wait for the 1.23 release to merge (the merge timing is up to you).

If the user is using a version before 1.23, they can use the call function method. If the user is using version 1.23 or above, they can use the for-range method.

See the implementation in commit 9489580edc4074268e457e47b1418544abf655c6.

Thanks.

My forked repo is pass the ci test at this commit. ref Screenshot 2024-07-24 at 11 59 22 AM

amikai avatar Jul 24 '24 03:07 amikai

Thanks for taking the time to do this, but I prefer to wait for the 1.23 release and not keep those conditional compiled files around - these tend to be useful for a short time as Go users typically upgrade quite fast to the latest versions. This will also allow enforcing the right toolchain in go.mod. Also, I think it's a bit nicer to use the iter.Seq2 type and the actual for-range in the test, as that's how this is expected to be used.

But don't worry about it, I'll just revert the latest commit when I merge the PR!

mna avatar Jul 24 '24 14:07 mna

I saw that Go 1.23 has been released, I'll try to get this merged later this week.

mna avatar Aug 13 '24 22:08 mna

Hi @mna You might have forgotten, so let me remind you because I want to use this feature 😂.

amikai avatar Sep 06 '24 09:09 amikai

@amikai yes, thanks for the ping! I'll try to release this today!

mna avatar Sep 06 '24 14:09 mna