Differential D24135

Fix missing directory separators when saving podcasts to disk
ClosedPublic
Actions

Authored by jyasi on Sep 21 2019, 3:32 PM.

Details

Reviewers
heikobecker
Group Reviewers
Amarok
Commits
R181:982772be0d06: Fix missing directory separators when saving podcasts to disk
Summary

SqlPodcastProvider::checkEnclosureLocallyAvailable() and SqlPodcastProvider::downloadResult() strip the trailing slash from the podcast download path, but do not add a directory separator when appending the download filename back to the directory. As a result, downloaded podcast filenames are prefixed with the directory name and stored in the parent directory.

Diff Detail

Repository
R181 Amarok
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jyasi created this revision.Sep 21 2019, 3:32 PM
Restricted Application removed a project: Amarok. · View Herald TranscriptSep 21 2019, 3:32 PM
Restricted Application added a subscriber: amarok-devel. · View Herald Transcript
jyasi requested review of this revision.Sep 21 2019, 3:32 PM
yurchor added a subscriber: yurchor.Sep 21 2019, 7:33 PM

+1
Tested to work as expected.

wbauer added a subscriber: wbauer.Sep 22 2019, 7:09 AM

You should add '/' instead of QDir::separator()...

https://agateau.com/2015/qdir-separator-considered-harmful/

jyasi updated this revision to Diff 66626.Sep 22 2019, 4:49 PM

Update based on feedback. Just use '/' instead of QDir::separator()

wbauer added a comment.Oct 3 2019, 9:06 AM

+1 from me now as well.

The KDE4 code uses KUrl::toLocalFile( KUrl::AddTrailingSlash ) here, so the additional slash should be correct (and necessary).

wbauer added a subscriber: Amarok.Oct 3 2019, 9:07 AM
heikobecker accepted this revision.Oct 3 2019, 6:58 PM
heikobecker added a subscriber: heikobecker.

Do you have commit access or should I push this for you?

This revision is now accepted and ready to land.Oct 3 2019, 6:58 PM
jyasi added a comment.Oct 4 2019, 1:00 AM
In D24135#541657, @heikobecker wrote:

Do you have commit access or should I push this for you?

I don't have commit access. Please push for me. Thanks!

Closed by commit R181:982772be0d06: Fix missing directory separators when saving podcasts to disk (authored by yurchor). · Explain WhyOct 4 2019, 6:26 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 67299

View Options

src/core-impl/podcasts/sql/SqlPodcastProvider.cpp

Loading...