feat: add AllPages pagination helper

[costela]

This small PR finally addresses #1320, picking up where the discussion left off.

Naming is - of course - up for discussion! :)

Update: the PR now also includes an experimental iterator based on the rangefunc experiment.

[costela]

Oh, this is embarrassing, I totally missed #1741 when searching for previous work :confused:

This implementation is considerably different though, so I'll leave the PR open for now.

[Roemer]

Looks cool but what I am missing from my PR (https://github.com/xanzy/go-gitlab/pull/1741) is the ability to 1. somehow allow the pagination to abort on some criteria and 2. the ability to immediately process each result. Both are a bit of edge cases but can come in handy in a few situations.

[costela]

I guess the iterator approach basically solves both your points? We could reduce this PR to just the iterator; just for those who explicitly want to test the new rangefunc stuff?

[Roemer]

Ah true, I misread that. Seems that the iterator approach could really do those things. That would be fantastic and way less clunky than my previous version.

[costela]

Just for the record: note that the iterator doesn't technically require the rangefunc experiment or go1.22, but I think we should still "hide" it behind the experiment to limit API breakage to those explicity interested.

If go1.23 turns out to break the rangefunc experiment API, we can then change the iterator with minimal user impact.

[JordanP]

It took me at least 5min to understand this line func PageIterator[O, T any](f Paginatable[O, T], opt *O) iter.Seq2[*T, error] { but it's beautiful !

[costela]

Just 5min? Then you were faster than me! 😅

Seriously though, I think renaming the type parameters to something more verbose may help?

And I think as long as the usage is straightforward (which IMHO it still is), then the implementation is allowed to look a bit gnarly.

As a side-note, I'm personally still a bit torn on the rangefunc stuff. Powerful and elegant (at least from a runtime perspective) but twists my brain in ways I'm not yet comfortable with.

[JordanP]

As a side-note, I'm personally still a bit torn on the rangefunc stuff. Powerful and elegant (at least from a runtime perspective) but twists my brain in ways I'm not yet comfortable with.

Yep ! For me, that feeling started with the introduction of generics. As a "user"/consumer, they are a joy to use. As a code author/maintainer, they are a pain.

---

Some functions take an additional pid interface{} as first argument (like Pipelines.ListProjectPipelines(...)). So I need a function like

func XXYYY[O, T any](pid interface{}, f func(interface{}, *O, ...gitlab.RequestOptionFunc) ([]*T, *gitlab.Response, error)) Paginatable[O, T] {
    return func(opt *O, optionFunc ...gitlab.RequestOptionFunc) ([]*T, *gitlab.Response, error) {
        return f(pid, opt, optionFunc...)
    }
}

It's a function that returns a closure, whose signature matches type Paginatable[O, T any]

Would that make sense to include such function in that gitlab package ?Is there a better way ?

[costela]

It's a function that returns a closure, whose signature matches type Paginatable[O, T any]

Would that make sense to include such function in that gitlab package ?Is there a better way ?

Also a good point. However, I'm not sure about the closure approach. It's elegant, but I fear it may not be directly clear for consumers. I've added an alternative suggestion to the code: *ForID variants to the pagination helpers. IMHO that's slightly more ergonomic for users, but I'll gladly refactor into the closure approach if that's the preferred way.

The two versions unfortunately only cover around 60% of list-like methods: if my grep-foo is correct, there are currently 187 list-like methods in total, 25 of the "simple" opt+optFunc type and 85 of the id+opt+optFunc type. Hopefully that's still enough of an improvement to warrant including it?

[costela]

Friendly reminder :innocent:

Now that the iter package is apparently being released as GA in 1.23 (at least it's in master and mentioned in the current draft release notes), it may be a good idea to reconsider this approach?

[costela]

And now 1.23 is out with rangefunc support :tada:

Any feedback here? :innocent:

[jakubbortlik]

And now 1.23 is out with rangefunc support 🎉

Any feedback here? 😇

Yeah, it would indeed be very nice if a pagination helper was implemented. Either this one or #1741.

It would also make it possible to close one more MRs - #1498 :-)