Finish the pagination implementation

[marioortizmanero]

Description

This finishes what was left at #166

Closes #124

[marioortizmanero]

I've had to make some parameters a reference, like Market, because when using automatic pagination we iterate multiple times and would have to clone the parameter.

[ramsayleung]

Yes, it makes sense. As I mentioned in another PR, it might be a chance to pass parameters by reference

[marioortizmanero]

While I was at it I made these parameters consistent; limit and offset always go last.

[marioortizmanero]

I've also tried to implement a macro for these but completely failed at doing so. It's really complicated with macro_rules; if anything we would need a proc macro, and even in that case it might not be worth it. I'll just leave it for the future.

[marioortizmanero]

I've added a pagination_chunks parameter to the client that configures the second parameter of pagination instead of hardcoding it to 50.

[marioortizmanero]

This should be ready for review now, @ramsayleung

[ramsayleung]

Is it necessary to have an anonymous lifetime annotation?

[marioortizmanero]

Yup, otherwise you get this:

   Compiling rspotify v0.10.0 (/home/mario/Programming/rspotify)
error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirem
ent
    --> src/client.rs:1005:13
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> {
     |                                      ----- this data with an anonymous lifetime `'_`...
1004 |         paginate(
1005 |             move |limit, offset| self.current_user_saved_albums_manual(Some(limit), Some(offset)),
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...is captured here...
     |
note: ...and is required to live as long as `'static` here
    --> src/client.rs:1003:48
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> {
     |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to declare that the `impl Trait` captures data from argument `self`, you can add an explicit `'_` l
ifetime bound
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> + '_ {
     |                                                                                         ^^^^

error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirem
ent
    --> src/client.rs:1005:39
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> {
     |                                      ----- this data with an anonymous lifetime `'_`...
1004 |         paginate(
1005 |             move |limit, offset| self.current_user_saved_albums_manual(Some(limit), Some(offset)),
     |                                  ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |                                  |
     |                                  ...is captured here...
     |
note: ...and is required to live as long as `'static` here
    --> src/client.rs:1003:48
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> {
     |                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to declare that the `impl Trait` captures data from argument `self`, you can add an explicit `'_` l
ifetime bound
     |
1003 |     pub fn current_user_saved_albums(&self) -> impl Paginator<ClientResult<SavedAlbum>> + '_ {
     |                                                                                         ^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0759`.
error: could not compile `rspotify`

To learn more, run the command again with --verbose.

[marioortizmanero]

Wait, but I can remove it from the paginate function. Give me a sec.

[ramsayleung]

The self has an anonymous lifetime '_, in order to explicitly indicate the return value has can live as long as the self, so we just simply add an '_ for the return value? is it right?

[marioortizmanero]

Yup, the thing is that paginate captures self, which has a borrowed lifetime of '_. Thus, we must indicate that the returned value won't outlive self with + '_. I thought this was done automatically by the compiler but being an impl it makes sense that it's forcing us to specify the lifetime.

[ramsayleung]

Beside the small nitpicks I commented, everything else looks good to me. And I think it's ready to merge :)

[marioortizmanero]

Ok @ramsayleung I've relaxed the lifetimes as much as possible and the tests have passed. Take a look and merge if you're ok with this PR then.

[ramsayleung]

Merged :)

[icewind1991]

Thanks for picking this up while I was busy/distracted!

[marioortizmanero]

No problem! If you have any concerns/suggestions about the implementation details, do let us know :)

[ramsayleung]

Thanks @icewind1991 and @marioortizmanero again for your great works :)

I just wrote a post to demonstrate this brilliant feature and how it works https://0x709394.me/Let's-make%20everything%20iterable

[marioortizmanero]

Great! I didn't know you had a blog, saved it. I plan on making one myself when we finally release 0.11 at https://nullderef.com :D

I enjoyed it, very visual. A few comments:

• Shouldn't the fetch diagram increase the offset by 4 instead of by 1 per request?
• Typo: First of all, the itrator pattern allows we -> First of all, the iterator pattern allows us

[ramsayleung]

Great! I didn't know you had a blog, saved it. I plan on making one myself when we finally release 0.11 at nullderef.com :D

Yep, we are stepping forward to v0.11. If you don't mind, I would like to add your blog to my blogroll :)

A few comments:

Oooh, Great catch, thanks for your heads-up.

[marioortizmanero]

Yep, we are stepping forward to v0.11. If you don't mind, I would like to add your blog to my blogroll :)

Cool! I don't have one yet but I'll definitely add you too when I do :)