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

List:       kde-devel
Subject:    Re: Incubation Request: Kiview
From:       christoph () cullmann ! io
Date:       2024-04-29 11:18:31
Message-ID: b61cd4e5e1939ed76367c37ece2a3e92 () cullmann ! io
[Download RAW message or body]

On 2024-04-29 09:21, Sune Vuorela wrote:
> On 2024-04-28, Méven <meven29@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
[prev in list] [next in list] [prev in thread] [next in thread]