Bug 166549 - amarok does not recurse directories when adding media to playlist
Summary: amarok does not recurse directories when adding media to playlist
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 2.0-SVN
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-14 16:44 UTC by David
Modified: 2009-12-09 11:28 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
When opening files, selected directories are added recursively (3.01 KB, patch)
2008-08-26 19:16 UTC, Andreas Mützel
Details
Fixes the Problem with Ian's solution. (1.31 KB, application/octet-stream)
2008-09-08 00:39 UTC, Andreas Mützel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David 2008-07-14 16:44:02 UTC
Version:           svn (using Devel)
Installed from:    Compiled sources
OS:                Linux

As the topic says, amarok 2 (from recent trunk), when adding new media, if a folder is selected, the folder (and not its contents) is added to playlist, and the result is a bad playlist not playable.
Way to reproduce:
1.- start amarok
2.- select Amarok->PlayMedia OR Playlist->AddMedia (both with same result)
3.- select one or more folders (without opening them)
4.- click OK
5.- See the new playlist broken.
Comment 1 Mark Kretschmann 2008-07-16 10:48:20 UTC
Yes, I can reproduce this.
Comment 2 Lydia Pintscher 2008-08-20 12:10:07 UTC
Is this still a problem in current SVN?
Comment 3 David 2008-08-20 15:03:55 UTC
I have recently compiled revision 849896 and I still confirm this bug. Even modifying 3 and opening those folders and clicking OK does not work.
Comment 4 Andreas Mützel 2008-08-26 19:14:51 UTC
I wrote a patch that adds a "tracksForUrlsRecursive"-function to the CollectionManager, which is then used for loading files via the "Add/Play Media..."-dialogs and via command-line arguments.
The new function seemed necessary because I didn't want to change the default "tracksForUrls"-Function, because it might be useful in some other case, i.e. if recursive loading is explicitly not wanted.
Comment 5 Andreas Mützel 2008-08-26 19:16:50 UTC
Created attachment 27070 [details]
When opening files, selected directories are added recursively
Comment 6 Ian Monroe 2008-08-28 02:35:52 UTC
Fixed with r853599.
Comment 7 Ian Monroe 2008-08-28 02:42:42 UTC
Heh, um, I wish I had noticed that patch. Started fixing this about the same 
time it was submitted.

The patch does use processEvents, so I guess my solution is more stable.
Comment 8 David 2008-08-28 15:30:20 UTC
I reopen this bug since it's partially solved (I'm testing with rev 853695 wich is > than 853599.

Imagine two scenarios: 
A:
folder1
  |
  +---f1
  |    |
  |    +--- 1.mp3
  |    +--- 2.mp3
  +---f2
  |    |
  |    +--- a.mp3
  |    +--- b.mp3
  +---f3
       |
       +--- x.mp3
       +--- y.mp3

B:
folder1
  |
  +---1.mp3
  +---2.mp3
  +---3.mp3

The case B is solved now when trying to load "folder1", but the case of A is not. If you try in case A to load "folder1" directly, you will get an empty playlist.

  
Comment 9 Andreas Mützel 2008-08-28 17:56:51 UTC
My patch should handle both cases, so feel free to try it.
Both solutions look good to me, so i don't mind if you don't like mine ;-)
Comment 10 Lydia Pintscher 2008-09-05 21:16:56 UTC
What is the status of this?
Comment 11 Andreas Mützel 2008-09-07 20:35:02 UTC
It seems like the KIO-Job that is used for listing all files is not really recursive... KIO::listRecursive is used in DirectoryLoader.cpp to get the contents of every selected url and their subdirectories, but it seems to recurse only into the first subdirectory of any selected url.
That leads to the behaviour mentioned by David.
I could not find a fix for this problem, is this maybe a bug in KIO::listRecursive? 
Comment 12 Andreas Mützel 2008-09-08 00:39:25 UTC
Created attachment 27302 [details]
Fixes the Problem with Ian's solution.

I was wrong, it wasn't a bug in KIO, there was only a minor error in the DirectoryLoader. I wrote a patch which works for me, could someone else please have a look and check it in if it is ok?
Comment 13 Mark Kretschmann 2008-09-08 08:34:44 UTC
Thanks Andreas, fixed with SVN commit r858442.
Comment 14 David 2008-09-30 00:27:48 UTC
I reopen, since the behavior now is even worse than what it was.
Now it has a huge problem: when only a file is selected, it loads recursively all.
I think the problem is trivial to solve by doing a check before doing the recursive call, but I wanted to notify it anyway.

To test: have a folder with subfolders and files in it, and try to load only one file within it, you will see all loaded in playlist.
Comment 15 Andreas Mützel 2008-10-08 20:06:19 UTC
Which revision are you using? This seems to work fine with r866504. 
(My testcase: 1 directory w/ 7 songs and 7 subdirectories in it. I can add any combination of subdirectories and songs and only the selected items are added to the playlist.)
Comment 16 Mark Kretschmann 2008-10-09 09:14:37 UTC
Works just fine here with latest svn. Closing report.