-
Notifications
You must be signed in to change notification settings - Fork 214
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
qt5 based implementation #28
Conversation
just for reference (as no premake is there) - here is the cmake build script that i'm using.
|
0e0cf45
to
5c32c48
Compare
note: this is not integrated into the build - I have no idea of premake in combination with qt but I still hope it can serve as a reference
{ | ||
const QString& entry = selectedFiles.at( 0 ); | ||
const size_t len = entry.size(); | ||
const QByteArray& ba = entry.toLatin1(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong, the output should be utf8
I see the primary benefit of NFD being avoiding large libraries like QT. If you are linking QT, why would you want NFD? |
There is also gtk; ) |
Unfortunately, Qt's Cmake is incompatible with our build system. I will not be maintaining Cmake or incorporating it into NFD. Pre-generating build scripts with Premake as we do is far better for our users. I have no idea how to maintainably support Qt via Premake and I find the rationale for Qt's existence in NFD at all circumspect, as one of the main goals is to allow library users to avoid linking Qt or Wx. There is also a question of license compatibility which I am not even going to render an opinion on at this point. I am going to pass on this PR. Thanks for giving it a shot and paying attention to NFD though. |
note: this is not integrated into the build - I have no idea of premake in combination with qt
but I still hope it can serve as a reference
I tried to match your coding style - if something isn't how you wanna it to be, please let me know.