D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added inline comments. INLINE COMMENTS > wbauer wrote in FindMySQLe.cmake:45 > Right, that's exactly what I was referring to in my last comment, and what > the openSUSE patch I mentioned (which originally was taken from Arch) is > supposed to fix. > > I.e. instead of `set(MYSQLE_LIBRARI

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added inline comments. INLINE COMMENTS > malteveerman wrote in FindMySQLe.cmake:45 > This breaks embedded for me. The embedded storage plugin silently links > against libmysqlclient and then fails when initializing. > It's a problem that exists independently of this revision but I thought

D24413: Fix Increase/Decrease Volume shortcuts

wbauer closed this revision. wbauer added a comment. Commited with https://invent.kde.org/multimedia/amarok/commit/87c56f8a130b2ddf183922015ce5bb31173bd912 REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24413 To: wbauer, #amarok, schweingruber Cc: heikobecker, schwe

D24413: Fix Increase/Decrease Volume shortcuts

wbauer added a comment. Herald added a project: Amarok. Ok, as it was accepted, and there were no further comments since my last question whether I should change it, I'm just going to commit it now. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24413 To: wbauer, #a

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added a comment. One thing I should note though: we do use a patch to fix things on openSUSE currently, but that's not a problem of this change. I plan to propose our patch upstream anyway (I originally took it from Arch Linux and didn't really understand why it is needed), althou

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added a comment. Sorry if you were waiting for me. It's good to go in from my side. I actually built mariadb-connector-c without mysql support here as a test, and it still worked fine. Although, there has been a change to master meanwhile that you probably should integrate: h

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added a comment. In D28284#634644 , @wbauer wrote: > On openSUSE, it doesn't enable MySQLe anymore: > >-- Found MySQL: -L/usr/lib64 -lmariadb > /usr/bin/mysql_config: unrecognized option '--libmysqld-libs' > -- Performing

D28284: Add FindMariaDB.cmake module and use it if MySQL is not found

wbauer added a comment. On openSUSE, it doesn't enable MySQLe anymore: -- Found MySQL: -L/usr/lib64 -lmariadb /usr/bin/mysql_config: unrecognized option '--libmysqld-libs' -- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONNECTION -- Performing Test HAVE_MYSQL_OPT_EMBEDDED_CONN

D28165: Fix compatibility with modern MySQL

wbauer added a comment. In D28165#634060 , @asturmlechner wrote: > In D28165#633145 , @wbauer wrote: > > > The content of my /usr/include/mysql/mysql_version.h (which is just a symlink to mariadb_ve

D28165: Fix compatibility with modern MySQL

wbauer added a comment. In D28165#634404 , @asturmlechner wrote: > Waiting for @wbauer to confirm it works as well Yes, the latest version builds fine here (the previous did as well), and the embedded collection still works as well.

D28165: Fix compatibility with modern MySQL

wbauer added a comment. This breaks compilation here on openSUSE with MariaDB: In file included from /home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20200314T055024~d790424e56/src/core-impl/storage/sql/mysql-shared/MySqlStorage.cpp:31: /usr/include/mysql/mysql.h:39:14: error: conflicti

D26008: Include the gmock prefix in the search path

wbauer accepted this revision. wbauer added a comment. This revision is now accepted and ready to land. Seems to be fine. It still finds gmock here on openSUSE and builds successfully. REPOSITORY R181 Amarok BRANCH master REVISION DETAIL https://phabricator.kde.org/D26008 To: tcbern

D24413: Fix Increase/Decrease Volume shortcuts

wbauer added a comment. In D24413#550948 , @heikobecker wrote: > I'm always easy to confuse about pointers and const, but isn't ec a const pointer to a const object? So calling ec->increaseVolume() as a non-const function) wouldn't really work

D24413: Fix Increase/Decrease Volume shortcuts

wbauer added a comment. Ping? Btw, I mainly followed what was done for the stop() action in commit 1b81541e , where a new `regularStop()` slot was created for similar reasons AIUI. Further suggestions how to

D24413: Fix Increase/Decrease Volume shortcuts

wbauer added a comment. In D24413#542035 , @ognarb wrote: > Since the method is only one line long, why not using lambda instead here? Because I couldn't get it to compile successfully, I got this error: In lambda function passing 'const

D24413: Fix Increase/Decrease Volume shortcuts

wbauer created this revision. wbauer added a reviewer: Amarok. wbauer added a project: Amarok. Herald removed a project: Amarok. wbauer requested review of this revision. REVISION SUMMARY `EngineController::increaseVolume( int ticks )` and `EngineController::decreaseVolume( int ticks )` take an

D24413: Fix Increase/Decrease Volume shortcuts

wbauer added a comment. PS: I think the argument is 0 because QAction::triggered() has a `bool checked` argument. But Qt's new signal/slots syntax doesn't support default arguments anyway, according to the docs. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24413

D24381: Register Play/Pause as global shortcut again

This revision was automatically updated to reflect the committed changes. Closed by commit R181:5ed62f9a089d: Register Play/Pause as global shortcut again (authored by wbauer). REPOSITORY R181 Amarok CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24381?vs=67245&id=67317 REVISION DET

D24381: Register Play/Pause as global shortcut again

wbauer added a comment. Btw, I just found a bug report about this: https://bugs.kde.org/show_bug.cgi?id=373590 I'll add a reference to the summary when I commit it. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24381 To: wbauer, #amarok, schweingruber, heikob

D24381: Register Play/Pause as global shortcut again

wbauer retitled this revision from "Register Play/Pause global shortcut again" to "Register Play/Pause as global shortcut again". REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24381 To: wbauer, #amarok Cc: amarok-devel, #amarok

D24381: Register Play/Pause global shortcut again

wbauer edited the test plan for this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24381 To: wbauer, #amarok Cc: amarok-devel, #amarok

D24381: Register Play/Pause global shortcut again

wbauer edited the test plan for this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24381 To: wbauer, #amarok Cc: amarok-devel, #amarok

D24135: Fix missing directory separators when saving podcasts to disk

wbauer added a subscriber: Amarok. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24135 To: jyasi, #amarok Cc: #amarok, wbauer, yurchor, amarok-devel

D24135: Fix missing directory separators when saving podcasts to disk

wbauer added a comment. +1 from me now as well. The KDE4 code uses `KUrl::toLocalFile( KUrl::AddTrailingSlash )` here, so the additional slash should be correct (and necessary). REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24135 To: jyasi, #amarok Cc: wbauer,

D24381: Register Play/Pause global shortcut again

wbauer created this revision. wbauer added a reviewer: Amarok. wbauer added a project: Amarok. Herald removed a project: Amarok. wbauer requested review of this revision. REVISION SUMMARY Commit 0b2b243b removed the ca

D24381: Register Play/Pause global shortcut again

wbauer edited the summary of this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24381 To: wbauer, #amarok Cc: amarok-devel, #amarok

D24135: Fix missing directory separators when saving podcasts to disk

wbauer added a comment. You should add '/' instead of QDir::separator()... https://agateau.com/2015/qdir-separator-considered-harmful/ REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24135 To: jyasi, #amarok Cc: wbauer, yurchor, amarok-devel

D24040: Don't delete whole folder when deleting a track

This revision was automatically updated to reflect the committed changes. Closed by commit R181:b986f52d1d53: Don't delete whole folder when deleting a track (authored by wbauer). REPOSITORY R181 Amarok CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24040?vs=66355&id=66527 REVISION

D24040: Don't delete whole folder when deleting a track

wbauer added a comment. In D24040#534325 , @schweingruber wrote: > Looks OK. What is the status of the unit tests, though? No idea. But this isn't tested at all AFAICS. Btw, this function is only used inside this source file, when

D24043: [TagDialog] Really enable "Open in filemanager" button for local files

This revision was automatically updated to reflect the committed changes. Closed by commit R181:decd2a3ecb31: [TagDialog] Really enable "Open in filemanager" button for local files (authored by wbauer). REPOSITORY R181 Amarok CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24043?vs=66

D24043: [TagDialog] Really enable "Open in filemanager" button for local files

wbauer added a subscriber: Amarok. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24043 To: wbauer, #amarok Cc: #amarok, amarok-devel

D24040: Don't delete whole folder when deleting a track

wbauer added a subscriber: Amarok. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24040 To: wbauer, #amarok Cc: #amarok, amarok-devel

D24043: [TagDialog] Really enable "Open in filemanager" button for local files

wbauer edited the test plan for this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24043 To: wbauer, #amarok Cc: amarok-devel

D24043: [TagDialog] Really enable "Open in filemanager" button for local files

wbauer created this revision. wbauer added a reviewer: Amarok. wbauer added a project: Amarok. Herald removed a project: Amarok. Herald added a subscriber: amarok-devel. wbauer requested review of this revision. REVISION SUMMARY `QUrl::QUrl()` expects a full Url including the scheme (unlike `KUr

D24040: Don't delete whole folder when deleting a track

wbauer edited the test plan for this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D24040 To: wbauer, #amarok Cc: amarok-devel

D24040: Don't delete whole folder when deleting a track

wbauer created this revision. wbauer added a reviewer: Amarok. wbauer added a project: Amarok. Herald removed a project: Amarok. Herald added a subscriber: amarok-devel. wbauer requested review of this revision. REVISION SUMMARY `SqlCollectionLocation::moodFile()` tries to replace the filename's

D21478: Fix passing local file paths on the command line

This revision was automatically updated to reflect the committed changes. wbauer marked an inline comment as done. Closed by commit R181:cea336303aaf: Fix passing local file paths on the command line (authored by wbauer). REPOSITORY R181 Amarok CHANGES SINCE LAST UPDATE https://phabricator.k

D21478: Fix passing local file paths on the command line

wbauer marked an inline comment as done. wbauer added inline comments. INLINE COMMENTS > wbauer wrote in App.cpp:280 > It does handle relative paths just fine, see the test plan. (I just tried > `./foo.ogg` to be sure) > QUrl::fromUserInput() passes the directory to QDir(), which assumes '.' if

D21478: Fix passing local file paths on the command line

wbauer added inline comments. INLINE COMMENTS > fvogt wrote in App.cpp:280 > Did it previously also handle relative paths such as `./foo.ogg`? If so, > passing `QDir::currentPath()` instead of a null string is AFAICT necessary. It does handle relative paths just fine, see the test plan. (I just

D21478: Fix passing local file paths on the command line

wbauer edited the test plan for this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D21478 To: wbauer, #amarok Cc: amarok-devel

D21478: Fix passing local file paths on the command line

wbauer edited the summary of this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D21478 To: wbauer, #amarok Cc: amarok-devel

D21478: Fix passing local file paths on the command line

wbauer edited the summary of this revision. REPOSITORY R181 Amarok REVISION DETAIL https://phabricator.kde.org/D21478 To: wbauer, #amarok Cc: amarok-devel

D21478: Fix passing local file paths on the command line

wbauer created this revision. wbauer added a reviewer: Amarok. wbauer added a project: Amarok. Herald removed a project: Amarok. Herald added a subscriber: amarok-devel. wbauer requested review of this revision. REVISION SUMMARY Use QUrl::fromUserInput() to handle local file paths (without "file