D21878: Rewrite servicemenu helper utility in C++
Elvis Angelaccio
noreply at phabricator.kde.org
Sun Jul 7 20:50:55 BST 2019
elvisangelaccio added a comment.
In D21878#491061 <https://phabricator.kde.org/D21878#491061>, @aspotashev wrote:
> > src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp
> > 35–42
> > Since we are in a Qt program now, we could just open a KMessageBox rather than spawn another process just to show an error dialog (and kdialog might not even be installed).
> >
> > 71–77
> > Same here. We should use QMimeDatabase rather than run xdg-mime.
>
> This patch is about rewrite to C++. What you suggest are good ideas, but can be done as separate patches, from my POV they are of lower priority.
>
> Besides the two issues you mentioned, the current implementation also has a huge bug that stops many services menus from being installed: the installation scripts are run from a wrong current working dir. I already have a patch to fix it, however I will post it when this one lands because otherwise it's hard to publish the second patch with Phabricator.
>
> To be exact, KMessageBox is not equivalent to `kdialog --passivepopup`.
Fair enough.
INLINE COMMENTS
> servicemenuinstaller.cpp:31
> +// @param msg Error that gets logged to CLI
> +[[noreturn]] void fail(const QString &str)
> +{
How about `Q_NORETURN` ?
REPOSITORY
R318 Dolphin
REVISION DETAIL
https://phabricator.kde.org/D21878
To: aspotashev, sitter, elvisangelaccio, ngraham
Cc: cfeck, kfm-devel, fprice, fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190707/f95c3afc/attachment.htm>
More information about the kfm-devel
mailing list