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
Lint Skipped
Unit
Unit Tests Skipped
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 66626

View Options

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

Loading...