sitter added a comment.
I have zero background knowledge here, but it really feels like there must be
an existing solution to this problem other than libmagic. Like how does kate
figure out the encoding of a text file.
INLINE COMMENTS
> Findlibmagic.cmake:1
> +# - Try to find libssh
> +# Once done this will define
bad copypaste
> Findlibmagic.cmake:64
> +
> +
> +endif (LIBMAGIC_INCLUDE_DIR AND LIBMAGIC_LIBRARIES)
It's more idiomatic to define an imported target, see some of the newer finders
in ECM.
I do also rather wonder if we couldn't just use FindPkgConfig's imported
target, in theory pkgconfig also works on windows. I may well be ignorant of
the finer points of windows support though.
> textcreator.cpp:38
> +#include <config-thumbnail.h>
> +#if LIBMAGIC_FOUND
> + #include "magic.h"
TBH, I would make libmagic required for building the thumbnail plugin. I can't
see much of a rationale for why we'd want to support "broken"/insufficient
encoding detection when there's code that makes it better.
> textcreator.cpp:65
> +#if LIBMAGIC_FOUND
> +static QTextCodec* codecFromFile(const QString &path)
> +{
`*` on wrong side of space
> textcreator.cpp:67
> +{
> + magic_t m = magic_open(MAGIC_MIME_ENCODING);
> + magic_load(m, nullptr);
better name than m? (:
> textcreator.cpp:69
> + magic_load(m, nullptr);
> + const char *ret = magic_file(m, path.toLocal8Bit() );
> + auto codecName = QString(ret).toUpper().toLocal8Bit();
excess whitespace towards the end. I also wonder if qfile::encodename would be
better
> textcreator.cpp:70
> + const char *ret = magic_file(m, path.toLocal8Bit() );
> + auto codecName = QString(ret).toUpper().toLocal8Bit();
> + magic_close(m);
I guess you could just toUpper on a QBA instead of going through a temporary
QString since ret is an encoding identifier ajnd would be always an ascii
string.
Also, can be const it seems.
> textcreator.cpp:78
> + if (strcmp(ret, "unknown-8bit")) {
> + // use latin for unkwnwn 8bit as it is quite versatile
> + return QTextCodec::codecForName("latin-1");
unkwnwn typo
REPOSITORY
R320 KIO Extras
REVISION DETAIL
https://phabricator.kde.org/D29381
To: meven, #frameworks, sitter, ngraham
Cc: pino, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio,
aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew,
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns,
emmanuelp, rdieter, mikesomov
