Incubation Request: Kiview

christoph at cullmann.io christoph at cullmann.io
Mon Apr 29 12:18:31 BST 2024


On 2024-04-29 09:21, Sune Vuorela wrote:
> On 2024-04-28, Méven <meven29 at gmail.com> wrote:
>> The immediate goal with this application is to fill this request 
>> feature
>> for dolphin :
>> https://bugs.kde.org/show_bug.cgi?id=272539
>> And we can imagine reusing it in many other places potentially.
> 
> I just opened 'DocumentViewer' class and spent 5 minutes and got a bit
> scared.
> 
> "instant" preview?
> 
> Launching a background libreoffice?
> Doing weird command line parsing of libreoffice? and bash pipe grep ?
> Then throwing a generated pdf at qtwebengine and hoping the best?
> 
> There is a lot of std::tsring to qstring and back again conversions
> 
> There are plenty of std::string deepcopies.
> 
> Who deletes ConversionThread ?
> 
> If this class is in any way representative of the code quality of the
> app, I really think we should reconsider.
> 
> If this class is not representative, then it definitely should be
> architecturally re-done.

Hi,

I gave it a quick look, too, it does stuff like

             // Unfortunately this is the only way I've found to close 
the Flatpak
             // version of libreoffice.
             std::string command =
                 R"(kill $( ps -aux | grep -F "/soffice.bin" | grep -F " 
--headless --nolockcheck --norestore --convert-to pdf" | grep -F " 
--outdir"| awk '{ print $2 }'))";
             QProcess process;
             process.setProgram(QStringLiteral("bash"));
             process.setArguments(QStringList() << QStringLiteral("-c") 
<< QString().fromStdString(command.c_str()));


That will just then kill any kind of libreoffice instances or perhaps 
even inject other commands depending what this grep matches.

I tend to agree that this would rather need some major overhaul.

Greetings
Christoph

> 
> /Sune


More information about the kde-devel mailing list