Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#18 improve the stored playlist editor

open
nobody
None
5
2011-07-09
2011-07-09
Justus Winter
No

Debian bug #506413 [0] suggests that arios stored playlist editing capabilities could be improved and it is indeed not possible to edit a playlist without replacing the primary play queue, editing the playlist there and resaving it.

As a first step I propose the attached patchset. It allows one to delete an arbitrary selection of songs (from an arbitrary set of selected playlists).

The patchset applies cleanly to ario-trunk + prevent-ario_mpd_emit_storedplaylist-from-being-called-repeatedly.patch from [1]. Given a big enough fuzz factor it also applies to trunk without my patch, but the bug renders the newly gained functionality unusable.

The author of the bug reports mentions gmpc, which can also cut, copy and paste. If you agree that this kind of functionality is an improvement I'm gonna look into that as well.

Comments and criticism are welcome, I'm new to the glib and gtk world :)

0: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=506413
1: https://sourceforge.net/tracker/?func=detail&aid=3359990&group_id=197448&atid=961512

Discussion

  • Marc Pavot
    Marc Pavot
    2011-07-17

    Hi,

    Thanks for the patch but I think you have forgotten some code changes. I get a compilation error because ario_server_playlist_delete_songs is not defined.

    Regards,
    Marc

     
  • Marc Pavot
    Marc Pavot
    2011-07-24

    Hi,

    Sorry for my previous message. I have missed two of your three patch files...

    Your patch is really interesting but I have some remarks to do before integrating it:
    - SONGS_POSITION_COLUMN can easily be avoided by using gtk_tree_path_get_indices method in songlists_foreach_get_playlist_position method. It is always better when you can avoid adding a new column to this kind of model.
    - Why are you using a hash table in songlists_foreach_get_playlist_position? Hash tables are useful when you need to access dynamically a value but here you are only looping sequentially on it. GSList is probably more adapted to what you want to do.
    - In compare funciton you are actually comparing pointers and not integer values. Do you really need to have a sorted list?

    Apart from these few comments, it is very good patch and thank you for respecting coding style of Ario!

    Regards,
    Marc

     
  • Justus Winter
    Justus Winter
    2011-07-24

    Hey Marc,

    thanks for your remarks, I'll take them into account when time permits it. I also wanted to ask you not to merge this patch right now as I want to clean it up some more, I'm working on a follow up patch that introduces copy and paste functionality and found that my naming conventions will introduce a conflict in the future. I also noticed that there is also a function mpd_send_playlist_delete that allows deletion requests to be batched.

    Many thanks for creating ario in the first place :)
    Justus