[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