After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 335185 - Split RBLibrarySource in two
Split RBLibrarySource in two
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: general
HEAD
Other Linux
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-03-20 05:01 UTC by James "Doc" Livingston
Modified: 2006-03-31 03:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (76.49 KB, patch)
2006-03-20 05:06 UTC, James "Doc" Livingston
none Details | Review
updated patch (78.44 KB, patch)
2006-03-29 05:03 UTC, James "Doc" Livingston
committed Details | Review

Description James "Doc" Livingston 2006-03-20 05:01:50 UTC
RBLibrarySource currently contains the "entry type browsing" bits and the actual Library bits. It would be good to split the former off into it's own class that other classes can derive from it without having the Library-specific stuff.
Comment 1 James "Doc" Livingston 2006-03-20 05:06:16 UTC
Created attachment 61588 [details] [review]
patch

Splits a lot of the RBLibrarySource code into RBBrowsableTypeSource, which culd probablt do with a better name.
Comment 2 Alex Lancaster 2006-03-21 05:58:44 UTC
Compiles fine.  Mostly looks good.

However by default it appears that the CD source has a browser.
Comment 3 Alex Lancaster 2006-03-28 10:08:28 UTC
Any chance this could be committed soon?  Seems to be working (with the exception of having a browser for the CD source).  Also would seem to be necessary before bug #322268 being committed and is useful in it's own right because many of the columns in DAAP and other sources shouldn't be there in any case (e.g. rating).
Comment 4 James "Doc" Livingston 2006-03-29 05:03:30 UTC
Created attachment 62264 [details] [review]
updated patch

Updated to cvs, and with cd browser issue fixed. I'm fairly sure I've got all the RBLibrarySource changes put into the correct place.

I wouldn't mind getting this committed if there are no problems, since keeping it updated with RBLibrarySource changes in cvs is annoying.
Comment 5 Alex Lancaster 2006-03-29 09:56:08 UTC
Works for me.
Comment 6 Jonathan Matthew 2006-03-29 11:03:32 UTC
Looks OK to me.  RBBrowsableTypeSource does need a better name, though - 'browsable' looks like a spelling mistake to me, and 'browseable' doesn't look any better.  RBBrowserSource?  RBSourceWithBrowser?
Comment 7 James "Doc" Livingston 2006-03-31 03:17:20 UTC
Committed to cvs, with the class renamed to RBBrowserSource.
Comment 8 Alex Lancaster 2006-03-31 03:46:42 UTC
Crash after updating from CVS:

0x00901638 in gtk_widget_show () from /usr/lib/libgtk-x11-2.0.so.0
(gdb) bt
  • #0 gtk_widget_show
    from /usr/lib/libgtk-x11-2.0.so.0
  • #1 rb_shell_sync_toolbar_visibility
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell.c line 2400
  • #2 rb_shell_constructor
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell.c line 1259
  • #3 g_object_newv
    from /usr/lib/libgobject-2.0.so.0
  • #4 g_object_new_valist
    from /usr/lib/libgobject-2.0.so.0
  • #5 g_object_new
    from /usr/lib/libgobject-2.0.so.0
  • #6 rb_shell_new
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell.c line 951
  • #7 main
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c line 325

Comment 9 Alex Lancaster 2006-03-31 03:48:53 UTC
Was a CVS conflict that I hadn't spotted.  Closing.  Sorry for the noise.