Comment 9 for bug 248769

Revision history for this message
dynamotwain (dynamotwain) wrote :

The bundle contains a series of commits that more or less can be applied individually. The later three directly deal with positioning and don't depend too much on the prior commits.

The changes sorted by my opinion of 'invasiveness', from most to least:
1. Updated docs files: Some types might have been purposely excluded in the past because they were considered 'internal'-only, despite being exposed from the library.

2. GObject-ify AwnSettings: It needs to be done at some point.
    * Having a nice singleton to maintain settings in a central location is a nice convenience. Having to break ABI for each setting you change or remove is not.
    * GTK+ has hit this same issue, which has prevented them from adding some features because it changes struts defined in public headers.
   * Accessor functions or GObject properties don't have this limitation.
   * A central settings manager that refreshes external updates is no good if its clients have to poll it for changes. GObject's "notify" signal is a trivially implemented solution that fits in with the rest of the GOBject-based codebase
  * ABI for the AwnSettings struct has already been broken twice in the past 4 months
  * Theoretically, since a maximum of one instance of AwnSettings will exist, there is no harm to not pairing g_object_unref calls with awn_settings_new calls; it's just a convenience for memleak debugging with Valgrind.
  * The next step would be to create accessors or properties for all the fields in AwnWidget and seal the members of the struct with GSEAL a'la GTK. Then after a major release with GSEAL enabled to give external applets time to do things right, move all the settings fields in the struct to a private struct instead of exposing it to the world.

3: Showing panel later after position is set:
   * Don't show the window before setting the position ourselves
   * Otherwise WM position AWN in some random place (possibly depending on mouse position) before we immediately move it to another location.
   * GTK was just going to put the window on the default screen anyway, so get the default screen without requiring the window to be realized first.

4. Use monitor.x and monitor.y when positioning windows
   * The change is simple: add monitor.x to the x value, and similar for y.
   * It's only so invasive because there's so much duplicated code in static functions in each of the .c files for the various windows.

5: Removal of AwnMonitor: It's not in libawn, but only in the source tree for one window that wasn't actually even using it.

6: Remove deprecated functions: the alternatives are functional equivalents and have been around since Gtk 2.0

7. Awn Applet dialog placement: use the monitor bounding box rather than 0..screenWidth

8. Deprecate awn_get_settings or not. Having two functions that do the exact same thing is pointless. Deprecate it like GTK+ does and leave it in, but conditionally compiled. This way you don't break ABI, but people know better than to use it and have a better chance of their code working as-is in the future.

9. autogen.sh changes: harmless unless you expect autogen.sh --prefix /usr to put files in /usr/local

All in all, the only API and ABI incompatible changes I made were in removing the force_monitor, monitor_width, and monitor_height fields from the AwnSettings struct, none of which any external applet has an reason to depend on, especially considering they had a tendency to be unrealistic forced values for TwinView/Xinerama users.

I didn't even touch upon the other ugliness I found in the source tree. There are static functions in src/ that were copy-pasted from one file to another that should be in say src/awn-window-shared.c for awn-bar, awn-hotspot, awn-main, awn-panel, and awn-window to use. Worse, there are non-singleton GObjects that store instance-specific values as global values! Even worse, some of them aren't static and pollute the global namespace. In the end, such clutter only makes programs harder to maintain and make changes like these appear more invasive than they really are.