On Sun, Jun 05, 2005 at 09:24:40PM +0200, Christophe Fergeau wrote:
> Hi,
>
> Le vendredi 03 juin 2005 � 09:13 +1000, Jonathan Matthew a �crit :
> > > Fwiw, I grabbed jonathan's --bugs-- branch as separate patches and
> > > started to review them individually, I have selected 60/70% of those
> > > which can go into mainline as is, and I still have to look more
> > > carefully at the other ones.
> >
> > Thanks! I wasn't looking forward to going through all of that.
> > Please let me know if there's anything I can do to help here. I know my
> > patch summaries aren't very descriptive, so I might need to clarify some
> > things.
>
> I put the patches that look fine to me at
> http://cfergeau.free.fr/rb-patches/
>
> http://cfergeau.free.fr/rb-patches/remaining/ contains the patches I
> still need to look at (unsurprisingly, those are the biggest ones ;)
I'll try to explain what I was thinking when I made each of these.
patch-5:
effectively undone by patch-6, as far as I can tell.
patch-6:
The playlist hashtable was getting out of sync with the query
model when entries were dropped into the playlist, since that
didn't go through rb_playlist_source_add_location. By updating
the hashtable in signal handlers for the query model row
insert/delete signals, it's always in sync.
patch-9:
obsolete, fixed (better) in CVS.
patch-12:
Rather than indirectly linking librb.la (and some others)
multiple times, just get the link order right. This makes
partial rebuilds work properly (I was having some weird problems
before this), and also eliminates a lot of link warnings on some
platforms.
patch-13:
Without this, rhythmbox ignores a lot of shoutcast streams
because it can't stat the stream - the gnome_vfs_get_file_info
call fails with GNOME_VFS_ERROR_GENERIC. I don't like it, but
at least it works.
patch-15:
Allows iradio streams to be added via drag and drop from
browsers (_NETSCAPE_URL) and elsewhere. Doesn't use rb-tree-dnd
because you shouldn't be able to reorder the stations, it's just
there so you can drag things in from outside.
(bug #158717)
patch-21:
Extracts the list of genres from the current iradio stations to
populate the genre field in the new iradio station dialog.
Also changes the new station dialog to only require the stream
location, not the genre or title, because those can be read from
the stream itself.
patch-22:
Just an experiment. Uses natural sort order for album titles,
so if you have albums numbered 'fnord vol. 1' through 'fnord vol. 20',
they get sorted into ascending order as if you'd zero-filled the
numbers. Doesn't really make much difference.
(bug #158599)
patch-25:
Currently, the menu popup shown by the sourcelist when you
right-click on it is the popup for the selected source, not the
source the pointer is over. This is inconsistent with the rest
of the universe, where right clicking on a non-selected item
selects the item and then shows the popup.
Adds popups for the iradio source (to add a new station) and the
blank space in the source list (to create new playlists).
I'm not sure what the change to rb-shell-player.c is doing
there. It should be removed.
(bug #131340)
patch-31:
Currently, clicking on a playlist name allows you to rename it.
It's easy to do this accidentally, and it's not really a common
thing to do. This change removes this behaviour and adds an
item to the menu popup to do renames. This makes playlist
renaming work like file renaming in nautilus.
(bug #133845)
patch-35:
Only update the rhythmdb entry for an iradio station if it's
actually been edited. I'm not sure if updating the db entry was
causing any problems, but there's no point doing it anyway if
there are no changes.
patch-36:
Show popups in the entry view when shift-f10 or menu key is
pressed, for consistency with nautilus etc.
(bug #152650 (well, half of it, at least))
patch-37, patch-41:
If there are multiple screens (not Xinerama), the tray icon is
only shown on the one rhythmbox is running on. This was an
attempt at fixing that, but I don't think it was entirely
successful. This should probably be left out until I get it
right.
patch-38:
Changes the way signals are emitted from
rhythmdb_query_model_entry_changed_cb so when an entry is
changed such that it no longer matches the query, signal
handlers can determine what actually happened. I'm not 100%
sure, but I think this fixes the "crash on editing iradio
properties" bugs that keep getting reported (#160358).
The query model now emits the entry-prop-changed signal, then
a new entry-removed signal (which is different from row-deleted
as it indicates that the entry has only been filtered out by the
query, not deleted).
Prior to this, the property model could not update its
value reference counts correctly when a changed entry no longer
matched the query, because it never got the 'old' property
value. This usually caused crashes.
patch-40:
Adds a playback error display to the iradio station properties
dialog, to match the playback error display in the song
properties dialog.
patch-43:
Attempts to improve the tray icon's tracking of the main window
state, so it can remember whether the main window was maximized
before it was hidden. (#145268)
> The only patch I didn't like so far is patch-10, see below for the
> notes I took at the time :
>
> --- orig/configure.ac +++ mod/configure.ac @@ -38,6 +38,8 @@
>
> PKG_PROG_PKG_CONFIG
>
> +PKG_PROG_PKG_CONFIG +
>
> Unneeded
Probably just the result of a botched merge, since I (stupidly) first
made these changes in another branch and merged them in.
> +if USE_CD_BURNER
> +librbplayer_la_SOURCES += \
> + rb-recorder.h \
> + rb-recorder-gst.c
> +
> +INCLUDES += $(LIBNAUTILUS_BURN_CFLAGS)
> +
>
> This breaks the xine backend, which I removed from my playbin branch but
> still seems to be present in the bug branch, so it's not really a big
> issue, but still ;)
Not breaking the xine backend wasn't particularly high on my list of
priorities..
thanks,
-jonathan.
_______________________________________________
rhythmbox-devel mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/rhythmbox-devel
