D29381: Thumbnail text: use libmagic to detect encoding

2020-05-20 Thread Harald Sitter
sitter added a comment. Browsing the code it looks like it mmaps the file though? And when I add some strategic sleeping I can verify that file goes towards shared memory. Oh, I suppose the trouble is that load gets called each ::create? Simply wrap it in a cpp class and static scope it

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-19 Thread Méven Car
meven added a comment. I am only half satisfied by the patch. Mostly because of libmagic `magic_load` that loads a 5M file each time which is not needed to detect encoding. I would add some tests before landing anyways. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricato

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-19 Thread Harald Sitter
sitter added a comment. LGTM. Seeing as I don't have much background knowledge I'm not comfortable accepting though. I guess if nobody comes up with better options by next week feel free to land. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D29381 To: meven, #

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-18 Thread Méven Car
meven updated this revision to Diff 83055. meven marked 5 inline comments as done. meven added a comment. Use QByteArray, find typo, code style and naming REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29381?vs=82252&id=83055 BRANCH arcpatch-D29381 R

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-08 Thread Méven Car
meven updated this revision to Diff 82252. meven added a comment. Oops REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29381?vs=82251&id=82252 BRANCH D29381 REVISION DETAIL https://phabricator.kde.org/D29381 AFFECTED FILES CMakeLists.txt cmake/

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-08 Thread Méven Car
meven updated this revision to Diff 82251. meven added a comment. Make Thumbnail ioslave devicePixelRatio capable REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29381?vs=81786&id=82251 BRANCH thumbnail-dpr REVISION DETAIL https://phabricator.kde.or

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-06 Thread Méven Car
meven added a comment. Perhaps it'd make sense to refactor this a bit and construct some test cases around encoding detection so we get a sense of reliablity? The way I am looking at this: either libmagic always does the best job at detecting encodings, at which point we'll want it

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-05 Thread Harald Sitter
sitter added inline comments. INLINE COMMENTS > meven wrote in textcreator.cpp:38 > Without libmagic, it is current state basically UTF-8 with bom detection > otherwise local codec. > > I did not test exhaustive encodings so I wanted to let the door open for > users to not rely on libmagic. >

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-05 Thread Méven Car
meven added inline comments. INLINE COMMENTS > sitter wrote in textcreator.cpp:38 > 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 >

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-04 Thread Harald Sitter
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 > +# On

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-04 Thread Méven Car
meven added a comment. In D29381#662084 , @pino wrote: > why not KEncodingProber from the KCodecs framework, like also the commented bits hint about? I tested first KEncodingProber, it just returns bad data, returning "UTF-8" sometimes f

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-03 Thread Pino Toscano
pino added a comment. why not KEncodingProber from the KCodecs framework, like also the commented bits hint about? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D29381 To: meven, #frameworks, sitter, ngraham Cc: pino, kde-frameworks-devel, kfm-devel, azyx, nikol

D29381: Thumbnail text: use libmagic to detect encoding

2020-05-03 Thread Méven Car
meven created this revision. meven added reviewers: Frameworks, sitter, ngraham. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY libmagic is the libraray used in `file` utility to sea