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

JlCompress is missing utf-8 support #114

Open
davidwojnar opened this issue Apr 2, 2021 · 4 comments
Open

JlCompress is missing utf-8 support #114

davidwojnar opened this issue Apr 2, 2021 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@davidwojnar
Copy link

It would be nice to be able to use the JlCompress utility function with utf-8 support.

It should be fairly simple to do that. Basically for each function one only has to do the following:
QuaZip zip(fileCompressed);
zip.setUtf8Enabled(true);

Backward compatibility makes it a little bit trickier.
So, there are at least four possible solutions for that:

  1. make it a compile time condition
  2. Add some sort of static variable to JlCompress that can be set e.g. static bool JlCompress::enableUtf8 = false;
  3. Add a new parameter to the functions e.g. static bool compressFile(QString fileCompressed, QString file, bool enableUtf8 = false);
  4. Add new functions e.g static bool compressFileUtf8(QString fileCompressed, QString file);

I can update the code, and send a pull request, but would like to have some opinion on which way to go.
I am preferring 3 the most, but am open for better suggestions.

@stachenov
Copy link
Owner

I'd suggest the 5th way:

Make JlCompress a full-fledged non-static class, with Pimpl and fluent setters. Delegate static functions to their non-static counterparts for backwards compatibility. Use non-static ones in new code, like

JlCompress().withUtf8Enabled().compressFile(...)

This way more options can be added later while maintaining full API and ABI compatibility.

A good naming scheme is needed to avoid name clashes between static and non-static functions. I can't think of anything good right away, save for silly ideas of capitalizing or underscoring them...

Or wait. Why JlCompress? Just create a new class. Delegate JlCompress methods to it. Pick a better name while we're at it, as JlCompress doesn't even follow the QuaSomething naming scheme. Let's call it QuaCompress for the sake of similarity or QuaUtil maybe...

@davidwojnar
Copy link
Author

I was actually hopeing for an easy solution to my problem, but you are right.

Sp I started moving some code around. And will replace JlCompress implementation with a class called QuaZipUtil. That way, JlCompress can still be used as is.

Will send a pull request once I have finished it and added some tests.

@Neustradamus
Copy link

@davidwojnar: Have you done a PR?

@cen1
Copy link
Collaborator

cen1 commented Oct 12, 2024

JlCompress functions have recently been extended with an Options class which could now contain this functionality without expanding the interface into oblivion. I'll consider patching this in when there's an opportunity. I also like the setter idea which could be added in parallel.

@cen1 cen1 added enhancement New feature or request good first issue Good for newcomers labels Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants