Re: [PATCH] add graph window

[Jody Goldberg <jody gnome org>]

> +GogObject   *gog_object_get_child_by_name(GogObject const *obj, const char *name);
please commit this with the minor stylistic adjustment that in
gnumeric/goffice we use
    char const *
in place of
    const char *

I prefer this idiom because it is clearer that it is the content
that is const rather than the pointer.

> --- goffice/gtk/Makefile.am   5 Sep 2005 21:02:55 -0000       1.30
> +++ goffice/gtk/Makefile.am   4 Nov 2005 11:46:36 -0000
> -     go-graph-widget.c
> +     go-graph-widget.c \
> +     \
> +     go-marshal.list
Please keep these in goffice/utils rather than creating a new list.
I realize that there is some extra overhead in the non-gtk enabled
build but it seems cleaner to keep them together.
 
> +             static const GEnumValue values [] = {
> +                     { GO_GRAPH_WIDGET_REFERENCE_DIRECTION_NONE, 
> "GO_GRAPH_WIDGET_REFERENCE_DIRECTION_NONE", "horizontal" },
This looks wrong.  Shouldn't that be "none" ?

> +                  (w->zoom_factor > 1.0 ?
> +                   w->height < w->width * w->aspect_ratio :
> +                   w->height > w->width * w->aspect_ratio))) {
What are you trying to achieve here ?

>  GtkWidget *
> -go_graph_widget_new (void)
> +go_graph_widget_new (GogGraph *graph)
Reasonable.

>  /**
> + * go_graph_widget_set_graph :
> + * @widget : #GOGraphWidget
> + * @graph: #GogGraph
> + * 
> + * Sets the #GogGraph embedded in the widget. May not be NULL.
Please add a comment that it adds a reference.  To me the most
important docs are always lifecycle related.
  
>   * go_graph_widget_get_chart :
>   * @widget : #GOGraphWidget
>   * 
> - * Returns the #GogChart created by go_graph_widget_new().
> + * Returns the #GogChart of the embedded #GogGraph.
ditto here now that I read it.

> --- goffice/gtk/go-graph-widget.h     8 Aug 2005 08:57:00 -0000       1.2
> +++ goffice/gtk/go-graph-widget.h     4 Nov 2005 11:46:37 -0000
> @@ -32,12 +32,23 @@ G_BEGIN_DECLS
> +typedef struct _GOGraphWidgetClass GOGraphWidgetClass;
Why make this public ?