- Also support MIME type "application/x-compressed-tar".
- Update tests in Ruby, remove SimpleCov.
BUG: 399229
BUG: 399229
Ruby tests passed
No Linters Available |
No Unit Test Coverage |
Buildable 12938 | |
Build 12956: arc lint + arc unit |
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. |
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.
src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp | ||
---|---|---|
31 | How about Q_NORETURN ? |
src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp | ||
---|---|---|
35–42 | I checked the sources of kdialog and discovered that what "kdialog --passivepopup" does is not trivial, see https://cgit.kde.org/kdialog.git/tree/src/kdialog.cpp#n517 Firstly it tries to show a Plasma/shell notification over D-Bus. If unavailable, it falls back to KPassivePopup. In the Beautiful KNewStuff of the Future, we could pass the error message back to the "Get New Stuff" dialog and display it there as an inline notification. One of the ways to implement this would be a plugin system where Dolphin's servicemenuinstaller is a plugin (.so/.dll). |
src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp | ||
---|---|---|
35–42 | I see, thanks for investigating it. |