[Bug 497853] Re: Support application indicators
Travis B. Hartwell
nafai at travishartwell.net
Mon Feb 8 22:44:44 GMT 2010
Patch was made against upstream git tag BRASERO_2_29_6, matching the
version in Lucid, and I have verified patch both cleanly applies against
the original tarball acquired via apt-get source -d brasero (using patch
-p0) and against git HEAD (via git rebase).
Feature complete with the following caveats. particularly to make it
upstream acceptable:
- Probably need to add a flag to configure like
--enable/disable-app-indicators. My autoconf foo is weak, so I left
that undone.
- When compiling with app-indicator support,
libbrasero-burn/brasero-tray is still compiled and linked, which is
unnecessary. I tried added #ifdefs in Makefile.am around the lines
which included brasero-tray.c and brasero-tray.h, but that didn't
work.
- I have code duplication between the existing
libbrasero-burn/brasero-tray.c and the new
libbrasero-burn/brasero-app-indicator.c, due to a cut and paste
job to get started. The areas I'm concerned about:
- in libbrasero-burn/brasero-app-indicator.c, the function
brasero_app_indicator_set_progress_menu_text (starting line 227)
is largely a copy of libbrasero-burn/brasero-tray.c, the function
brasero_tray_icon_set_tooltip (starting line 244), except for the
portion where the menu item label is set vs. the original tooltip
being set.
- similarly, libbrasero-burn/brasero-app-indicator.c, function
brasero_app_indicator_set_progress (starting line 281) is largely
a copy of libbrasero-burn/brasero-tray.c, function
brasero_tray_icon_set_progress (starting line 293), except for the
calls to set the tooltip and set the icon.
I wasn't sure how to address these the best. It didn't make sense
to inherit BraseroAppIndicator from BraseroTrayIcon because
BraseroTrayIcon itself is a sub-class of GtkStatusIcon. Should I
just move the common calculation / string creation methods out to a
separate .h/.c pair and call those from each? That leaves the
question of where to free the string.
Any other suggestions would be great so I can improve my patches in
the future.
** Patch added: "Patch to use Application Indicators"
http://launchpadlibrarian.net/38906062/app-indicators.patch
--
Support application indicators
https://bugs.launchpad.net/bugs/497853
You received this bug notification because you are a member of Ubuntu
Burning Team, which is subscribed to brasero in ubuntu.
More information about the Ubuntu-burning
mailing list