Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX page_token feature about messages, threads #39

Closed
wants to merge 1 commit into from

Conversation

kazuya030
Copy link
Contributor

page_token parameter changed to be used only in page_and_trim
internaly, and not accessed externally (messages, threads)

Paging seems to have a bug. This code can test it and should return 200, but now returns 100.

# should return 200
length(unique(id(messages(num_results = 200)))) 

This response to #19

page_token parameter changed to be used only in page_and_trim
internaly, and not accessed externally (messages, threads)
@jimhester
Copy link
Member

I ended up fixing this slightly more simply 94ba42c

length(unique(id(messages(num_results = 200)))) 
#> [1] 200

Thank you for the pull request, it let me know where the issue was!

@jimhester jimhester closed this Jan 2, 2016
@kazuya030
Copy link
Contributor Author

Thank you for your response.
However, the problem seems to exist yet.
For, example by below code. ids shold be larger than 100 and smaller than 1000

ids = id(messages(search = "in:sent -is:chat after:15-12-01 before:16-01-01", num_results = 1000))

then, master branch returns length(ids) = 1000
On the contrary, my branch kazuya030:fixPageToken returns ,length(ids) = 392, in my case.

I don't understand this clearly.
If you have an idea, please teach me, plrease.

Sincerely, yours.

@jimhester
Copy link
Member

You are correct the previous fix was incomplete, b48757a should now produce the same results with either fork.

@kazuya030
Copy link
Contributor Author

Thank youfor your quick response.
This libraly is very useful thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants