[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Incubation Request: Kiview
From:       Sven Brauch <mail () svenbrauch ! de>
Date:       2024-04-29 22:30:59
Message-ID: 60989db3-9588-4f6b-a026-84876e7b6b3f () svenbrauch ! de
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]

[Attachment #4 (multipart/mixed)]

[Attachment #6 (text/plain)]

Hi,

On 28.04.24 12:22, Méven wrote:
> Here is an email for incubating a new KDE project: Kiview.
> Kiview is a simple file previewer written in C++/Qml, from its README:

looking through the code there are unfortunately a lot of things one 
trips over.

First and foremost, as others have mentioned, there is way too much 
system() / shell stuff going on. This kind of code tends to be hard to 
get working reliably and easily creates security vulnerabilities.

Instead, I would approach the whole problem differently, by trying to 
leverage the tools KDE Frameworks already offers and uses. Why do you 
need to execute /usr/bin/unzip when KArchive offers this functionality? 
Why use WebEngine when there are already solutions in KDE for rendering 
PDF files? All this could be done in-process without involving 
filesystem operations or shell commands (and probably in a fraction of 
the computation time).

Do you know about the KParts interface? Have a look at that. I think 
it's pretty close to what you want already. E.g. Ark already has a very 
simple document previewer based on that.

Architectural concerns aside, here is a short list of things I noticed 
which should be improved:

  - Don't inherit QThread. Instead, use a more modern task interface 
such as QtConcurrent::run() or std::launch.

  - You do not need threads to asynchronously handle a QProcess. It has 
signals for that.

  - Numbers which have a fixed meaning should be an enum{}.

  - Don't write to files with a fixed name in /tmp. Multiple instances 
of your program will clash and you might overwrite user data. Tools like 
QTemporaryDir exist which avoid these problems.

  - What is going on with the clipboard stuff in dolphinbridge.cpp...? 
That looks like an abuse of the clipboard.

  - Prepending magic prefixes like !isDir! to paths isn't a proper 
solution for anything.

Hope this helps with the next iteration of this program.

Greetings,
Sven

["OpenPGP_0xA4AAD0019BE03F15.asc" (application/pgp-keys)]
["OpenPGP_signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]