Port audiocd-kio away from kdelibs4support
ClosedPublic

Authored by yurchor on Sep 22 2018, 8:09 PM.

Details

Summary

Just a guess on how it can be ported. Will it be enough just use the common argv[0], argv[1], argv[2] arguments to slave() without parsing?

Test Plan
  1. KIO can be compiled and installed.
  2. Dolphin understands audiocd:/ and the files from MP3, OGG, etc. subdirectories can be copied (extracted).
  3. K3b is not tested.

Diff Detail

Repository
R342 KIO AudioCD
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
yurchor requested review of this revision.Sep 22 2018, 8:09 PM
yurchor created this revision.
yurchor updated this revision to Diff 42165.Sep 23 2018, 7:09 AM

Rebase and get rid of whitespaces.

yurchor updated this revision to Diff 42678.Oct 1 2018, 4:09 PM
yurchor retitled this revision from Port audiocd-kio away from deprecated KApplication to Port audiocd-kio away from kdelibs4support.
yurchor edited reviewers, added: KDE Applications; removed: Dolphin.

Complete port to KF 5. Tested to work on audio CDs from my collection. Still unsure about the parsing of the command line in audiocd.cpp. Please help.

aacid added a subscriber: aacid.Oct 1 2018, 9:33 PM
aacid added inline comments.
audiocd.cpp
84

If this comment about libkcddb is true, this is going to be bad, needs to be a QApplication to be able to bring up widgets.

yurchor marked an inline comment as done.Oct 2 2018, 7:04 AM
yurchor added inline comments.Oct 2 2018, 7:09 AM
audiocd.cpp
84

It might be a very stupid question, but is it enough to change "QCoreApplication" to "QApplication"?

I have tried to do so, but even after adding

set(REQUIRED_QT_VERSION 5.9.0)

find_package(Qt5 ${REQUIRED_QT_VERSION} REQUIRED COMPONENTS Network Widgets)

to CMakeLists.txt, it refuses to find QApplication for "#include <QApplication>". I'm lost. :'(

yurchor updated this revision to Diff 42705.Oct 2 2018, 7:10 AM

Convert to generic KIO template

elvisangelaccio added inline comments.
audiocd.cpp
84

No, it's not enough. You also need to link against Qt5::Widgets in cmake (that will make the header file available for inclusion).

yurchor updated this revision to Diff 42750.Oct 2 2018, 7:52 PM

Switch to QApplication. Thanks for the hint.

yurchor marked 2 inline comments as done.Oct 2 2018, 7:54 PM
aacid accepted this revision.Oct 2 2018, 9:12 PM

I haven't tested it, but code seems sensible, so if you have tested it and are confident it works, i'd say go for it

This revision is now accepted and ready to land.Oct 2 2018, 9:12 PM
This revision was automatically updated to reflect the committed changes.