When trying to create a folder in the saved playlist^merged view I see a "New Folder" appear on the screen, but the output only gives me edit: editing failed When I rename that folder, the result on the konsole out put is X Error: BadWindow (invalid Window parameter) 3 Major opcode: 20 (X_GetProperty) Resource id: 0x1800407 Once I quit Amarok and restart it the made folders are gone.
Bart, this goes to you much probably. I scent something horribly wrong in PlaylistsInFoldersProxy or directly in starting-to-smell-real-bad QtGroupingProxy. This is essentially the same bug as 310532 (also a release_blocker). Because the "Playlists in folders feature" doesn't work for me at all (I haven't found a way to put a playlist into a folder) and the implementation is hacky, I'm thinking about removing it for 2.7, with a possibility for you to re-add it once fixed. Also, bug 301297 is closely related.
(In reply to comment #1) > This is essentially the same bug as 310532 (also a release_blocker). For the lazy: bug 310532.
(In reply to comment #1) > Bart, this goes to you much probably. I scent something horribly wrong in > PlaylistsInFoldersProxy or directly in starting-to-smell-real-bad > QtGroupingProxy. This is essentially the same bug as 310532 (also a > release_blocker). > > Because the "Playlists in folders feature" doesn't work for me at all (I > haven't found a way to put a playlist into a folder) and the implementation > is hacky, I'm thinking about removing it for 2.7, with a possibility for you > to re-add it once fixed. Also, bug 301297 is closely related. Don't remove it. Would be a huge regression for users that organized their playlists this way. It has always worked for SqlPlaylists, but empty folders are not kept. This is because a folder in PlaylistBrowser is really a label on a playlist, i.e. a property of a playlist. If there is no playlist without that property there is no folder.
(In reply to comment #3) > (In reply to comment #1) > > Because the "Playlists in folders feature" doesn't work for me at all (I > > haven't found a way to put a playlist into a folder) and the implementation > > is hacky, I'm thinking about removing it for 2.7, with a possibility for you > > to re-add it once fixed. Also, bug 301297 is closely related. > > Don't remove it. Would be a huge regression for users that organized their > playlists this way. It has always worked for SqlPlaylists, but empty folders > are not kept. Bart, are you actually able to put a Saved Playlist or a Podcast Channel into a newly created folder? With current git? I am not. Bart, if you would like to keep this feature, please fix regression release_blocker bug 250062, which I am able to reproduce every time by creating 3 new empty folders, trying to drag some saved playlists to them (doesn't work, they apparently don't accept drops) and the quitting Amarok. > This is because a folder in PlaylistBrowser is really a label > on a playlist, i.e. a property of a playlist. If there is no playlist > without that property there is no folder. Yep, I understand this. After 2.7 is released, we should provavly change the UI to read "labels" or something similar. There seems to be some implementation of nested folders, which I cannot test, and which seems extremely sloppy to me - what is the satus of it?
I'll look into in during the next week while on the train. It usually is something stupid that I can fix easily once I understand the (miss)use of the proxy again.
Git commit cc88eb136229be323e4c6c76ecc13ac6cfbf3ceb by Bart Cerneels. Committed on 24/12/2012 at 12:22. Pushed by shanachie into branch 'master'. Force editability of playlist folders. M +9 -0 src/browsers/playlistbrowser/PlaylistsInFoldersProxy.cpp M +2 -0 src/browsers/playlistbrowser/PlaylistsInFoldersProxy.h http://commits.kde.org/amarok/cc88eb136229be323e4c6c76ecc13ac6cfbf3ceb
Git commit e1cad5e80f8f40b08c0c46d85afe0450db2c497c by Bart Cerneels. Committed on 24/12/2012 at 12:33. Pushed by shanachie into branch 'master'. The correct edit() was not called, lacked polymorphism. M +1 -0 src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp M +2 -1 src/browsers/playlistbrowser/PlaylistBrowserCategory.h http://commits.kde.org/amarok/e1cad5e80f8f40b08c0c46d85afe0450db2c497c
Inline edits of folder names are still impossible. This might be be a Qt bug: https://bugreports.qt-project.org/browse/QTBUG-26838 I have not found a workaround yet. I think it's not this bug though, inline rename is working for playlists.
Git commit 607ce0c3c3b89df3c16e19273e796092e26d156b by Matěj Laitl. Committed on 31/12/2012 at 16:14. Pushed by laitl into branch 'master'. PrettyTreeView: proper polymorphism, revert e1cad5e80f8f40b0 Also revert "The correct edit() was not called, lacked polymorphism." This reverts commit e1cad5e80f8f40b08c0c46d85afe0450db2c497c. Bart, this is the right way to do it. Also, the polymorphism was not the cause of failing edit, see the next commit. CCMAIL: Bart Cerneels <bart.cerneels@kde.org> M +1 -2 src/browsers/playlistbrowser/PlaylistBrowserCategory.h M +6 -8 src/widgets/PrettyTreeView.cpp M +28 -24 src/widgets/PrettyTreeView.h http://commits.kde.org/amarok/607ce0c3c3b89df3c16e19273e796092e26d156b
Git commit 369508c70b7d9ced37afc0ddfdb661d72d17c3ab by Matěj Laitl. Committed on 31/12/2012 at 16:25. Pushed by laitl into branch 'master'. QtGroupingProxy: implement buddy() to work-around design issues This is effectively a work-around for a big design flaw in QtGroupingProxy (which is that it "invents" its own items not present in the original model). Technical description in the code comments. ^^^^ Bart, because of the above reason I'd prefer not to use QtGroupingProxy in Amarok at all in long-term. Instead, we should do it the other way around: original model would be hierarchical and the proxy would flatten it into a table. This fixes some bugs in the Saved Playlists and perhaps more: BUGFIXES: * Fix editability and drop-ability of playlist folders. CCMAIL: Bart Cerneels <bart.cerneels@kde.org> M +1 -0 ChangeLog M +0 -1 src/browsers/playlistbrowser/PlaylistBrowserCategory.cpp M +23 -0 src/browsers/playlistbrowser/QtGroupingProxy.cpp M +1 -0 src/browsers/playlistbrowser/QtGroupingProxy.h http://commits.kde.org/amarok/369508c70b7d9ced37afc0ddfdb661d72d17c3ab
Okay, this bug now boils down to preserving empty folders. This is a design issue of the Saved Playlists View: we internally store "labels" per each playlist, but represent them as folders in saved playlists. Because of that we have no way of storing a folder (label) with no associated playlist. Because of the above, the bug cannot be solved for 2.7, isn't now a regression and doesn't block the release at all IMO.
I would like to add that this bug affects populated folders too. I'm using 2.7.0 but noticed the problem in 2.6.0. When displaying the list of saved playlists in merged view, I can add a folder, rename the folder, and move playlists into the folder. All looks well until I restart Amarok. The folders are gone and the saved playlists reverts to a flat list. I miss this feature as this is the way I organized Amarok 1.4. I can "survive" with the flat list but having the folders is wonderfully convenient. :) I don't have a local git repo but if patches are made available I can merge them into a fresh build and help test.
A possible way to store folders without any playlists is to keep a "dummy playlist". This playlist will have the "label" as the name of the folder and a default name (or empty) and specific database ID common to all dummy playlists. The SqlPlaylistGroup will form the group as it does now. The PlaylistsInFoldersProxy will have to be changed to handle this accordingly and not display such playlists. It will only display the folder. Nested empty folders will be preserved in the same way. If this seems like a good idea I will make a patch implementing it.
Looks like it still not fixed in v2.8.0. Also when I am trying to delete 1 folder all folders are deleted.