Re: Gtk{Tree,List} replacement proposal

[Tim Janik <timj gtk org>]

On 3 Jul 2000, Jonathan Blandford wrote:

> Hi,
> 
> As some of you know, I've been working for the last few months on a
> replacement for the old GtkTree stuff (and in particular, a better
> CList,CTree).  I think I finally have the API far enough along that I
> can post it to the list for comments.

cool.

> First, a quick note about the design of the tree:
> 
> There are a number of objects associated with this:
>  GtkTLView - a view for trees/lists
>  GtkTLModel - a generic model for Trees/Lists.

why is this a GtkObject? are there any reasons for making it not directly
inherit from GObject?

> {
>         GtkObject *model;
>         GtkWidget *view, *window, *scroll;
> 
>         model = gtk_tree_model_new ();
>         my_init_model (model);
>         view = gtk_tlview_new_from_model (model);

note that we have an established convention to underscore-split capitalized
type names, encoded in the enum stringification macros in gtk's sources.
that is:

PrefixXOneLetter   -> prefix_xone_letter    -> PREFIX_XONE_ENUM_VALUE
PrefixXYTwoLetters -> prefix_xy_two_letters -> PREFIX_XY_TWO_ENUM_VALUE

so you'd either have to go for TLview (odd), something like ZView that uses
only one capitalized letter as prefix, or gtk_tl_view...

>         window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
>         scroll = gtk_scrolled_window_new (NULL, NULL);
>         gtk_container_add (GTK_CONTAINER (scroll), view);
>         gtk_container_add (GTK_CONTAINER (window), scroll);
> }


> ---------
> gtktlview.h
> ---------

> #define GTK_TYPE_TLVIEW			(gtk_tlview_get_type ())
> #define GTK_TLVIEW(obj)			(GTK_CHECK_CAST ((obj), GTK_TYPE_TLVIEW, GtkTLView))
> #define GTK_TLVIEW_CLASS(klass)		(GTK_CHECK_CLASS_CAST ((klass), GTK_TYPE_TLVIEW, GtkTLViewClass))
> #define GTK_IS_TLVIEW(obj)		(GTK_CHECK_TYPE ((obj), GTK_TYPE_TLVIEW))
> #define GTK_IS_TLVIEW_CLASS(klass)	(GTK_CHECK_CLASS_TYPE ((obj), GTK_TYPE_TLVIEW))

the _GET_CLASS() macro is missing

> typedef enum
> {
>   GTK_TLVIEW_COLUMN_RESIZEABLE,
>   GTK_TLVIEW_COLUMN_AUTOSIZE,
>   GTK_TLVIEW_COLUMN_FIXED
> } GtkTLViewColumnType;
> 
> typedef enum
> {
>   GTK_TLVIEW_SELECTION_SINGLE,
>   GTK_TLVIEW_SELECTION_MULTI
> } GtkTLViewSelectionType;
> 
> typedef struct _GtkTLView       GtkTLView;
> typedef struct _GtkTLViewClass  GtkTLViewClass;
> typedef void (*GtkTLViewForeachFunc) (GtkTLNode *node,
> 				      gpointer  data);
> typedef gboolean (*GtkTLViewSelectFunc) (GtkTLNode *node,
> 					 gpointer data);
> struct _GtkTLView
> {
>   GtkContainer parent;
> 
>   gpointer priv;
> };

why is all the view's stuff private? that disables derivation,
pretty unaccaptable for a widget as often used and tweaked as
list/tree views.

> struct _GtkTLViewClass
> {
>   GtkContainerClass parent_class;
> 
>   void (*set_scroll_adjustments) (GtkTLView     *tlview,
> 				  GtkAdjustment *hadjustment,
> 				  GtkAdjustment *vadjustment);
> };
> 
> GtkType        gtk_tlview_get_type                 (void);
> GtkWidget     *gtk_tlview_new                      (void);
> GtkWidget     *gtk_tlview_new_with_model           (GtkTLModel       *model);

please move those asterisks away from the function name, since owen and me can't
agree on using GtkWidget* for return types, simply make it:

GtkWidget *      gtk_tlview_new_with_model           (GtkTLModel       *model);


> /* Selection code */
> void           gtk_tlview_selection_set_type       (GtkTLView        *tlview,
> 						    GtkTLViewSelectionType type);

maciej already made appropriate coments on the naming here:

gtk_tlview_set_selection_type ()

> /* Options for manipulating the columns */
> void           gtk_tlview_column_set_visible       (GtkTLView        *tlview,
> 						    gint              column,
> 						    gboolean          visible);
> gboolean       gtk_tlview_column_get_visible       (GtkTLView        *tlview,
> 						    gint              column);
> void           gtk_tlview_column_set_type          (GtkTLView           *tlview,
> 						    gint                 column,
> 						    GtkTLViewColumnType  type);
> gint           gtk_tlview_column_get_type          (GtkTLView        *tlview,
> 						    gint              column);
> void           gtk_tlview_columns_set_type         (GtkTLView           *tlview,
> 						    GtkTLViewColumnType  type);
> gint           gtk_tlview_column_get_preferred_size(GtkTLView        *tlview,
> 						    gint              column);

make this one gtk_tlview_get_preferred_column_width(), since
gtk_tlview_get_column_preferred_width() sounds pretty odd.

> /* Actions */
> void           gtk_tlview_moveto                   (GtkTLView        *tlview,
> 						    GtkTLPath        *path,
> 						    gint              column,
> 						    gfloat            row_align,
> 						    gfloat            col_align);

i'd rather not repeate the clist misnaming here and see gtk_tlview_move_to(), as
far as my knowledge goes, "moveto" isn't a real word in english, right?

> ---------
> gtktlmodel.h
> ---------

> typedef gpointer                GtkTLNode;

what is this? i suppose you use a real GtkTLNode structure internally?
then use:
typedef struct _GtkTLNode GtkTLNode; and use GtkTLNode *node throughout
the API.

> typedef struct _GtkTLPath       GtkTLPath;
> typedef struct _GtkTLModel      GtkTLModel;
> typedef struct _GtkTLModelClass GtkTLModelClass;
> 
> struct _GtkTLModel
> {
>   GtkObject parent;
> };

so that should be GObject.

> struct _GtkTLModelClass
> {
>   GtkObjectClass parent_class;
> 
>   /* signals */
>   void       (* node_changed)    (GtkTLModel *TLModel, GtkTLPath  *path);
>   void       (* node_inserted)   (GtkTLModel *TLModel, GtkTLPath  *path);
>   void       (* node_deleted)    (GtkTLModel *TLModel, GtkTLPath  *path);
> 
>   /* VTable - not signals */
>   gint       (* get_columns)       (GtkTLModel *TLModel);
>   gchar     *(* get_column_header) (GtkTLModel *TLModel, gint      column);
> 
>   GtkTLPath *(* get_root_path)   (GtkTLModel *TLModel);

another strange looking variant of asteriskification ;)

> };

> /* Basic tlmodel operations */
> GtkType      gtk_tlmodel_get_type          (void);
> 
> /* GtkTLPath Operations */
> GtkTLPath   *gtk_tlpath_new                (void);
> void         gtk_tlpath_add_index          (GtkTLPath  *path,
> 					    gint        index);
> void         gtk_tlpath_prepend_index      (GtkTLPath  *path,
> 					    gint        index);
> gint         gtk_tlpath_get_depth          (GtkTLPath  *path);
> const gint * gtk_tlpath_get_indices        (GtkTLPath  *path);
> void         gtk_tlpath_free               (GtkTLPath  *path);
> GtkTLPath   *gtk_tlpath_copy               (GtkTLPath  *path);
> 
> /* Header operations */
> gint         gtk_tlmodel_get_columns       (GtkTLModel *tlmodel);

is this the number of columns, i.e. gtk_tlmodel_get_n_columns()?

> /* Path manipulations */
> GtkTLPath   *gtk_tlmodel_get_root_path     (GtkTLModel *tlmodel);
> 
> /* Node operations */
> GtkTLNode    gtk_tlmodel_get_node          (GtkTLModel *tlmodel,
> 					    GtkTLPath  *path);
> void         gtk_tlmodel_node_set_udata    (GtkTLModel *TLModel,
> 					    GtkTLNode   node,
> 					    gpointer    udata);
> gpointer     gtk_tlmodel_node_get_udata    (GtkTLModel *TLModel,
> 					    GtkTLNode   node);

this is for user data i suppose. if you spare that one pointer
per node anyway, make it a GData* pointer and use the quarked data
API, that will scale in the future and provide you with reliable
destroy notification out of the box (you can still have convenience
variants to wrap the "user-data" quarks for udata though).

> ---------
> gtktreemodel.h
> ---------

> #define GTK_TYPE_TREE_MODEL			(gtk_tree_model_get_type ())
> #define GTK_TREE_MODEL(obj)			(GTK_CHECK_CAST ((obj), GTK_TYPE_TREE_MODEL, GtkTreeModel))
> #define GTK_TREE_MODEL_CLASS(klass)		(GTK_CHECK_CLASS_CAST ((klass), GTK_TYPE_TREEMODEL, GtkTreeModelClass))
> #define GTK_IS_TREE_MODEL(obj)			(GTK_CHECK_TYPE ((obj), GTK_TYPE_TREE_MODEL))
> #define GTK_IS_TREE_MODEL_CLASS(klass)		(GTK_CHECK_CLASS_TYPE ((obj), GTK_TYPE_TREE_MODEL))

the _GET_CLASS() variant is missing.

> GtkType      gtk_tree_model_get_type           (void);
> GtkObject   *gtk_tree_model_new                (void);
> void         gtk_tree_model_set_columns        (GtkTreeModel   *model,
> 						gint            columns);
> void         gtk_tree_model_set_column_name    (GtkTreeModel   *model,
> 						gint            column,
> 						const gchar    *name);
> GtkTreeNode *gtk_tree_model_node_new           (void);
> void         gtk_tree_model_node_set_cell      (GtkTreeModel   *model,
> 						GtkTreeNode    *node,
> 						gint            column,
> 						GtkTLCell      *renderer,
> 						gpointer        data);

there's no destroy notification for the data here. couldn't the renderer
simply use quarked data with destroy notification ere as well, if it requires
the data pointer?

> ---------
> gtktlcell.h
> ---------

> #ifndef __GTK_TLCELL_H__
> #define __GTK_TLCELL_H__
> 
> #include <gtk/gtk.h>
> 
> #ifdef __cplusplus
> extern "C" {
> #pragma }
> #endif /* __cplusplus */
> 
> typedef enum
> {
>   GTK_TLCELL_SELECTED = 1 << 0,
>   GTK_TLCELL_PRELIT = 1 << 1,

make this GTK_TLCELL_PRELIGHT

>   GTK_TLCELL_INSENSITIVE = 1 << 2
> } GtkTLCellType;

> struct _GtkTLCellClass
> {
>   GtkObjectClass parent_class;
> 
>   /* vtable - not signals */
>   gint (* get_width)  (GtkTLCell *cell, gpointer   data);
>   gint (* get_height) (GtkTLCell *cell, gpointer   data);
>   void (* render)     (GtkTLCell *cell, GtkWidget *view,  GdkEventExpose *expose, GdkRectangle *area, gint x_offset, gint y_offset, guint flags, gpointer data);

hm, argument alignment wouldn't hurt ;)

ok, i just quickly glanced over it, it seems to be a good straight
forward approach so far. gotta have to look at some of the implementation
and use it a bit ;)

---
ciaoTJ