Fix documentation of getResults

[maneeshpm]

Fixes #596

I propose we should follow the same convention as used by Xapian for consistency. That is, rather than keeping the second argument of Search::getResults(start, end) as end of range, we should use maxResults as the second argument. This way:
getResult(start, maxResults) will return results in the range (start, start + maxResults)

This makes use of API much easier and will require just a documentation fix. We need to fix the rest of the projects accordingly(even though they still work and won't break after this change) for which issues are being created.

[codecov]

Codecov Report

Merging #597 (6a0bcbe) into master (947b66a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #597   +/-   ##
=======================================
  Coverage   82.78%   82.78%           
=======================================
  Files          91       91           
  Lines        3822     3822           
  Branches     1703     1703           
=======================================
  Hits         3164     3164           
  Misses        657      657           
  Partials        1        1           

Impacted Files	Coverage Δ
include/zim/search.h	100.00% <ø> (ø)
src/search.cpp	73.68% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 947b66a...6a0bcbe. Read the comment docs.

[kelson42]

@maneeshpm LGTM, see kiwix/libkiwix#597 as well

[mgautierfr]

Wow! It seems that it is not a simple documentation fix.

https://xapian.org/docs/apidoc/html/classXapian_1_1Enquire.html#a1f2e35f79bc731bebfbdaceda17b9892 indeed takes a maxResults, not a end.
So the previous code was buggy. We must check in all our user code how this is used. And add unit test getting range of results in the middle of the search to test this.

[maneeshpm]

@mgautierfr Another possible solution is, instead of changing any of our existing user code, we modify getResult(start, end). We can first find maxItems as end - start and use this in get_mset. We might have to check if the maxItems calculated like this is valid or not and throw an error accordingly. This would help to keep the issue confined within libzim. But we would have this inconsistency with Xapian get_mset which would have to be documented explicitly.

[kelson42]

@maneeshpm I'm not in favour of generating an exception here for that. Your fix seems proper to me, it just has to be tested properly. Keep in mind the libzim should be easy to use.

[maneeshpm]

@kelson42 I agree. We can go ahead and create corresponding tickets to "Check proper usage of getResults" in the rest of our projects and check them one by one.

[kelson42]

@maneeshpm Usually I'm not in favour to create test dedicated ticket excpet if there is a really good reason. Creating automated tests is part of the feature/bugfix development and multiplicatying tickets without reasons just makes things uselessly more complex. IMO it should be done in this PR, or do you see a really good reason to not do it now?

[maneeshpm]

@kelson42 I meant in the rest of our projects like kiwix-destop where this might be causing an issue, but we will have to check. I will add a test for libzim in this pr itself.

[kelson42]

@maneeshpm OK, I understand better. Once the PR is completed, would be great if you could check that the following repository are not impacted by a wrong assumption about end/maxResults:

• openzim/python-libzim
• openzim/node-libzim
• openzim/zim-tools (zimsearch)
• kiwix/libkiwix
• kiwix/kiwix-tools (kiiwx-serve)
• kiwix/kiwix-desktop
• kiwix/java-libkiwix

And if you see something just open a ticket then.

[kelson42]

@mgautierfr Can we merge this?