add tests for enqueuing files and fix one issue
Details
Details
- Reviewers
astippich - Group Reviewers
Elisa - Maniphest Tasks
- T7504: Add the possibility to enqueue arbitrary music files and add support for command line options
- Commits
- R255:b471e0330527: allow elisa to open external files, scan them and enqueue them
one can open a file by using "open with" or in a terminal by doing elisa musicFiles.ogg
Diff Detail
Diff Detail
- Repository
- R255 Elisa
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
I found two issues during testing:
when opening on command line with e.g. "elisa xxx.ogg", it can correctly read the metadata, but the entry becomes invalid when trying to start playback. with the absolute path e.g. "elisa /home/user/xxx.ogg" it works. Some handling of relative path is required.
playback is not started when the file is added, and elisa should probably jump to the added file in the playlist. but maybe that is something for later
Comment Actions
- add debug info to better understand cases where it may fail
- if the file exists, play list entry is valid immediately
- when adding tracks from command line arguments, immediately play them
try to fix comments and problems seen by astippich
Comment Actions
- add missing files
- add debug info to better understand cases where it may fail
- if the file exists, play list entry is valid immediately
- when adding tracks from command line arguments, immediately play them
- finish modification for display of play list entry from a random file
Comment Actions
Still unchanged behavior, but I think I've found the problem. The setArguments method for elisaapplication needs the same treatment for relative paths as activateRequested. I assume that the first is used for command line arguments while the latter is used for DBus signals, am I correct? I hacked up a working solution for me, which you find inline.
| src/MediaServer.qml | ||
|---|---|---|
| 186 | I changed that to var previousTrackNumber = playListModelItem.tracksCount enqueue(elisa.arguments) switchTo(previousTrackNumber) manageAudioPlayer.ensurePlay() to ensure playback of the added title similar to the enqueue signal. | |
| src/elisaapplication.cpp | ||
| 139 | Based on the activateRequested signal I quickly hacked: void ElisaApplication::setArguments(const QStringList &newArguments) QStringList filesToOpen;
QString workingDirectory = QDir::currentPath();
qDebug() << "ElisaApplication::activateRequested" << newArguments << workingDirectory;
for (const auto &oneFile : newArguments) {
auto newFile = QFileInfo(oneFile);
if (newFile.isRelative()) {
newFile = QFileInfo(workingDirectory + QStringLiteral("/") + oneFile);
}
if (newFile.exists()) {
filesToOpen.push_back(newFile.canonicalFilePath());
}
}
if (d->mArguments == filesToOpen) {
return;
}
qDebug() << "ElisaApplication::activateRequested" << filesToOpen;
d->mArguments = filesToOpen;
Q_EMIT argumentsChanged();} | |
Comment Actions
Thanks for the analysis. I had not understood the case that was failing.
I will update the review including automatic tests for ElisaApplication class.
Comment Actions
- always transform the music file names to aboslute ones in ElisaApplication
- when launching elisa with arguments, enqueue and play them immediately
Thanks a lot for your inputs and analysis work.