Font button

[Soeren Sandmann <sandmann daimi au dk>]

Matthias Clasen <maclas gmx de> writes:

> I've just attached new versions of these to
> http://bugzilla.gnome.org/show_bug.cgi?id=107682 and
> http://bugzilla.gnome.org/show_bug.cgi?id=107597
> 
> Changes wrt to the previous versions:
> - reduced some excessively long function names
> - moved to instance private data
> - improved documentation
> - fixed a few bugs noticed while doing the above
> 
> 
> I'm going to commit these in a few days, if nobody has further comments.

	- Why move to instatnce private data for a new widget?
	  I would think that the priv pointer is fine.

	- Why only two paddings in the class struct, not four as
          usual?

	- Is a destroy method really necessary?  Isn't it good enough
	  to just unref the font dialog in the finalize handler?

	- Don't we usually write "sans 12" as "Sans 12"?

	- Why are the parameters called "gfp" (GnomeFontPicker?)
	  "fb" would probably be better, or even "font_button"
	
	- What about a constructor new_with_font()?

	- The string setters do not handle the case where the new
	  string is equal to or a substring of the old string. Same
	  for other string setters.  I believe this is the correct
	  boiler place:

		saved_string = priv->string_property;

		priv->string_property = g_strdup (new_string);

		g_free (saved_string)

		...

		g_object_notify ();

	- boolean setters should only emit notification when the new
	  values are actually different from the previous one, to
	  minimize the risk of infinite loops.

	  The standard way to do this is:

		use_font = (use_font != FALSE)

		if (use_font != priv->use_font)
		{
			priv->use_font = use_font;

			...;

			g_object_notify ();

		}

	  With this the getters do not have to use 

		return priv->boolean_property? TRUE : FALSE;

	they can just return the property
		
	- There is a formatting problem in the else branch of
	  gtk_font_button_set_use_font()

Søren