added cutomIndexTemplate option to kiwix-serve

[MananJethwani]

this fixes #571

[codecov]

Codecov Report

Merging #607 (30e4c54) into master (e46b0c0) will decrease coverage by 2.86%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
- Coverage   66.10%   63.23%   -2.87%     
==========================================
  Files          53       53              
  Lines        3815     3452     -363     
  Branches     1913     1933      +20     
==========================================
- Hits         2522     2183     -339     
+ Misses       1291     1266      -25     
- Partials        2        3       +1     

Impacted Files	Coverage Δ
src/server/internalServer.h	100.00% <ø> (ø)
src/tools/pathTools.cpp	50.27% <66.66%> (-7.77%)	⬇️
include/server.h	100.00% <100.00%> (ø)
src/server.cpp	70.00% <100.00%> (-19.29%)	⬇️
src/server/internalServer.cpp	82.41% <100.00%> (-1.16%)	⬇️
src/tools/otherTools.cpp	82.56% <0.00%> (-9.99%)	⬇️
src/manager.cpp	75.78% <0.00%> (-2.20%)	⬇️
src/server/request_context.cpp	63.04% <0.00%> (-1.91%)	⬇️
... and 20 more

---

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 e46b0c0...30e4c54. Read the comment docs.

[MananJethwani]

@mgautierfr @veloman-yunkan, where can I add test for customIndexTemplate, and exactly how, are there any tests for other arguments like withLibraryButton which I can refer to.

[veloman-yunkan]

@mgautierfr @veloman-yunkan, where can I add test for customIndexTemplate, and exactly how, are there any tests for other arguments like withLibraryButton which I can refer to.

@MananJethwani I don't think there is such a test. I also believe that the best way to test this functionality would be in kiwix-tools, by running the kiwix-serve binary via pytest. Please see kiwix/kiwix-tools#375 for how I had intended to do something similar, before @kelson42 fairly pointed out that that feature better be tested in libkiwix via googletest.

[kelson42]

I want indeed to have the httpd daemon primitive tested within libkiwix and I don't understand what stop us to do it.

[kelson42]

I want indeed to have the httpd daemon primitive tested within libkiwix and I don't understand what stop us to do it.

[MananJethwani]

@kelson42 and @veloman-yunkan can you please have a look at the tests I have created, thanks.

[kelson42]

@MananJethwani We should add a document in the README about the template variables which are exposed.

[mgautierfr]

I wonder if this is not a bit too much.
The main need in #571 is to customize the style/text of what is displayed.
With this PR we allow the user to change almost everything. We the constraints that he must correctly do everything.
If he changes the js, he also must do the correctly js call to our server (or it own using our API). It introduces de facto a new public API on our side.

Maybe it is enough to let the user set the index title and allowing it to simply set a custom css ?
For now, this PR make me feel we are opening a Pandora's box.

[kelson42]

@MananJethwani Hi, any feedback here to Matthieu's comments?

[MananJethwani]

@kelson42 extremely sorry for the delay will update the PR by tomorrow

P.S. - sorry I actually missed the mail😅

[mgautierfr]

See my comments in the code.

You've adapted the test code but we don't really test the feature itself as we never try to run a test with a custom template.
A basic test would be to pass a custom template with a specific word/sentence and check that this word is correctly present in the output.

[MananJethwani]

@mgautierfr any updates?

[kelson42]

I have rebased the branch on origin/master

[mgautierfr]

We still need a bit more testing. To be sure we actually use the custom template index passed.

I think you can also simplify the git history. We probably need two commits :

• One to add the handling off the customIndexTemplate (and the associated tests)
• One to expose the 3 helper functions in tools.h

[MananJethwani]

@mgautierfr any updates?
I have made the changes as requested

[kelson42]

From a user perspective, works as expected.

[mgautierfr]

Yet a last easy fix in the test (I could have found this in previous review but miss it because of the double reset).

Everything else is good.