Introduced Formatter in libzim

[ShaopengLin]

Hi! I am a GSOC student this year and its my first PR here.

Changes:

• Added Formatter Class in src/tools.h. Revised comments to fit our purpose and changed the code from Issue #102: create ZIM redirect entry for file symlinks. zim-tools#158 to be more standard.
• Replaced majority of usages of (o)stringstream with Formatter with a few exceptions
• Removed imports that are no longer necessary to improve compilation time.

Discussion needed:

• I am not sure if we want the class to be public facing? If no, then the createZimExample.cpp should not be changed. The user would be confused what Formatter is.
• Classes with their own operator<< defined with std::ostream (e.g. blob.h) and functions that uses more complex features of stringstreams (e.g. class CapturedStderr) in my opinion should not need changes.
  • Making ourselves std::ostream goes against our goal to keep things in a stringstream.
  • Having the same featrues of stringstreams will likely mean inheritance.

Fixes #432

[mgautierfr]

Thanks for the PR.

Few things to change.
I'm in favor of using it in archive.cpp and debug.h too.

[codecov]

Codecov Report

Attention: Patch coverage is 12.17391% with 101 lines in your changes are missing coverage. Please review.

Project coverage is 58.04%. Comparing base (f624344) to head (c234ff2).

Files	Patch %	Lines
src/file_reader.cpp	0.00%	36 Missing and 1 partial ⚠️
src/writer/creator.cpp	9.09%	4 Missing and 6 partials ⚠️
src/narrowdown.h	0.00%	4 Missing and 5 partials ⚠️
src/writer/cluster.cpp	0.00%	3 Missing and 5 partials ⚠️
src/debug.h	0.00%	6 Missing ⚠️
src/entry.cpp	28.57%	2 Missing and 3 partials ⚠️
src/file_compound.cpp	0.00%	5 Missing ⚠️
src/writer/counterHandler.cpp	0.00%	0 Missing and 4 partials ⚠️
include/zim/error.h	0.00%	0 Missing and 3 partials ⚠️
src/version.cpp	0.00%	3 Missing ⚠️
... and 7 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
+ Coverage   57.73%   58.04%   +0.30%     
==========================================
  Files         100      101       +1     
  Lines        4619     4617       -2     
  Branches     1936     1921      -15     
==========================================
+ Hits         2667     2680      +13     
+ Misses        672      667       -5     
+ Partials     1280     1270      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ShaopengLin]

I am a little against using it in test/archive. The CapturedStderr Class is using rdbuf to read the stream buffer which our class is intended to hide. It makes the class too coupled. If we want to replace all uses of (o)stringstream with Formatter, we should be using inheritance instead. When they need specific features of stringstreams I think it is best they just use the original classes.

[ShaopengLin]

Also, I think we should place the class in include/zim/tools.h. After installation, error.h would not be able to use this class.

[ShaopengLin]

Resolved most of the comments and moved to zim/tools.h to ensure installation is correct. Likely need another patch depending on the discussion of std::endl vs '\n' and archive.cpp

[mgautierfr]

Still a small change to do about closing ".

I agree with the global code but please rework the commits. Once this PR will be merged, we will not care about the small mistake made during development and fixed during review. We are more interested with a clean git history.

I would suggest 3 commits (but feel free to assume your own pov on this) :

• Fix closing ".
• Introduce Formatter.
• Use formater

[ShaopengLin]

Fixed the compilation issue with __ostream_type from different platforms. @mgautierfr Can you run the CI again?

[ShaopengLin]

I am new to codecov. I believe I should just add test cases in the test folder that hit those code changes right? This might take a while...

[ShaopengLin]

Should we ignore the codecov atm? So these changes were not part of the covered sections. If we see the sentry report from other PRs, the exact sections are shown as missed. I do not think I have enough knowledge on how to reproduce all these error edge cases. @mgautierfr @kelson42

[mgautierfr]

Should we ignore the codecov atm?

Yes, the formater is used a lot to generate error message. Difficult to fully test.