GNOME Bugzilla – Bug 165583
Drag and drop artist, album, genre to a playlist
Last modified: 2005-06-12 17:53:38 UTC
This isn't possible currently. I think the right way to fix it is to add a text/uri-list target to the drag. This way, one can drop file managers too.
Created attachment 36991 [details] [review] patch to rb-property-view.c This patch adds a text/uri-list target to RBPropertyView. Multiple artists/genres/albums can be selected and dropped to playlists or to another application. It also fixes memory that wasn't freed in the part that handles the text/x-rhythmbox-* targets. Finally it updates the example for using rhythmdb queries.
Would anyone care to comment? I haven't had any answer from the mailing list either. This patch is useful, it works, it solves a shortcoming of rhythmbox's ui, and I wouldn't like to let it rot. Of course, if there are problems with it, I'd like to know about them too.
Stay tuned, I don't have net access at home at the moment, so I can't test it. It's on my list of rhythmbox patches to look at when I can, don't worry ;)
Actually I can at least nitpick on the code ;) * put all variable declarations at the beginning of a block * coding style for function calls is inconsistent (sometimes you use foo (bar); and sometimes foo(bar); ), the rest of the codebase uses foo (bar); * the rest of the code uses if (condition) { [...] } else { [...] } it would be nice to be consistent with that * query_model_cb seems to be embedded in the body of another function, please don't do that ;) I just applied your patch to a check out of rb-0.9 from mainline, and it doesn't seem to compile.. Did you test compilation with the default rhythmbox flags ? (in particular with -Wall -Werror -std=gnu89 -Wmissing-prototypes) Don't worry about the coding style nagging, I'll fix it when I really test the patch, it would be nice if the patch compiled though (even if I can fix it too when I test it if you prefer) ;)
I didn't really bother with CFGLAGS since I used the debian rules... This will make it compile: --- rb-property-view.c.orig 2005-02-08 14:00:34.659238687 +0100 +++ rb-property-view.c 2005-02-08 14:00:59.655725489 +0100 @@ -539,7 +539,12 @@ RHYTHMDB_QUERY_SUBQUERY, subquery, RHYTHMDB_QUERY_END); - + + static gboolean query_model_cb (GtkTreeModel *query_model, + GtkTreePath *path, + GtkTreeIter *iter, + gpointer data); + static gboolean query_model_cb (GtkTreeModel *query_model, GtkTreePath *path, GtkTreeIter *iter, About the nested func, if I move it out I need to declare an ad-hoc structure to hold db & reply, which is ugly but I think I prefer that to a copy-paste of the gtk_tree_model_foreach code. I think I'll finally add a few lines for the all case, too.
Created attachment 37186 [details] [review] patch to widgets/rb-property-view.c & rhythmdb/rhythmdb.h Here is a patch that fixes the coding style and handles dragging the all item. It removes the nested function as well.
ping! There's a fine patch waiting here.
Yep, sorry, it's on my list of things to review, but lack of net access at home doesn't make things really easy...
I was just going to add a feature request bug regarding this exact ability. Not knowing how to code (just an end user), I have a question: - Will this patch autocreate a new playlist if the selection is dragged to a blank space in the playlist sidebar? This would be a nice shortcut.
nope, not yet. The strange thing is I see there already is some code for this, but it isn't activated. In fact, this patch only modifies the sending side.
Ok, the patch looks ok to me. You added a comment saying that the current dnd functions don't work with a multiple selection, wouldn't it be pretty easy to make it to work? I made a few (not really interesting) changes to your patch, I'll attach a diff, I'll also attach the patch I committed to my 0.9 branch.
Created attachment 39538 [details] [review] Modifications I made compared to your patch.
Created attachment 39539 [details] [review] patch I committed to my 0.9 branch
Thanks. The part with the x-rhythmbox-* targets doesn't support multiple selection, I didn't bother at the time because they weren't used anyway (maybe in another branch?). If I do drag and drop to smart playlists, I'll add it.
If they aren't used, there isn't much point in supporting multiple selections indeed, we might as well get rid of them unless you need them for the smart playlist stuff.
Christophe, is this patch in CVS?
It is now.