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