Re: Comments on 95362 (tree dnd) patch
- From: Kristian Rietveld <kris gtk org>
- To: Owen Taylor <otaylor redhat com>
- Cc: gtk-devel-list gnome org
- Subject: Re: Comments on 95362 (tree dnd) patch
- Date: Sun, 17 Aug 2003 03:55:03 +0200
On Fri, 2003-08-15 at 02:31, Owen Taylor wrote:
> I'd consider this definitely a 2.4-only patch. First it's
> just long and touching a complex bit of code.
>
> But also, it does subtly change the behavior of GTK+ a bit
> and while I consider the changes "compatible enough",
> it's still better to keep 2.2.x releases as little
> suprising as possible.
>
> Yes, this leaves tree DND pretty much broken until we
> get 2.4.0 out ... well, more incentive to get 2.4.0 soon. :-)
>
> It's possible if we get this into HEAD and it works well
> for a while, we can consider a backport for 2.2.4.
I agree with all of that. For the stuff below, all code blocks/comments
which I've deleted are things which I agree with and have corrected
accordingly.
> ===
> + tree_view->priv->empty_view_drop = 1;
> ===
>
> Instead of using a flag for this, couldn't you just use
> set_drag_dest_row([0], BEFORE) to signal a drop into an empty view?
No, I can't. set_drag_dest_row() will try to create a row reference for
path "0" at some point. The row reference constructor checks if the row
actually exists in the model (by trying to get the iter using the path),
and because the model is empty this will fail. So the row reference will
not be created.
> ===
> + if (!gtk_tree_model_get_iter (model, &iter, path) ||
> + !gtk_tree_model_iter_next (model, &iter))
> + tree_view->priv->drop_append_mode = 1;
> else
> {
> + tree_view->priv->drop_append_mode = 0;
> + gtk_tree_path_next (path);
> + }
> }
> ===
>
> Couldn't you just immediately use a path one past
> the end rather than using drop_append_mode? The only place
No, this path will be stored in a row reference too, so the row must
exist in the model.
> that the path at the end is used, when calling
> gtk_tree_drag_dest_row_drop_possible(),
> seems like a bug. The path passed to
> gtk_tree_drag_dest_row_drop_possible() needs to match
> that passed to gtk_tree_drag_dest_drag_data_received().
Fixed.
>
> ===
> dest_row = get_dest_row (context);
>
> if (dest_row == NULL)
> - return;
> + {
> + if (tree_view->priv->empty_view_drop)
> + dest_row = gtk_tree_path_new_from_indices (0, -1);
> + else
> + return;
> + }
> ===
>
> I don't think this code will be hit even if with your
> current code; even if empty_view_drop() is set,
> get_dest_row() will be set.
It does get hit, because we don't store a path for the empty view drop.
Attached a revised patch. Owen, thanks for your thorough review of this
code.
thanks,
-Kris
Index: gtktreeprivate.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreeprivate.h,v
retrieving revision 1.56
diff -u -p -r1.56 gtktreeprivate.h
--- gtktreeprivate.h 27 May 2003 21:20:56 -0000 1.56
+++ gtktreeprivate.h 17 Aug 2003 01:53:21 -0000
@@ -184,6 +184,10 @@ struct _GtkTreeViewPrivate
/* hint to display rows in alternating colors */
guint has_rules : 1;
guint mark_rows_col_dirty : 1;
+
+ /* for DnD */
+ guint empty_view_drop : 1;
+ guint drop_append_mode : 1;
/* interactive search */
guint enable_search : 1;
Index: gtktreednd.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreednd.c,v
retrieving revision 1.7
diff -u -p -r1.7 gtktreednd.c
--- gtktreednd.c 14 Jan 2003 22:57:37 -0000 1.7
+++ gtktreednd.c 17 Aug 2003 01:53:21 -0000
@@ -313,6 +313,9 @@ gtk_tree_get_row_drag_data (GtkSelection
if (selection_data->target != gdk_atom_intern ("GTK_TREE_MODEL_ROW", FALSE))
return FALSE;
+ if (selection_data->length < 0)
+ return FALSE;
+
trd = (void*) selection_data->data;
if (tree_model)
Index: gtktreeview.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreeview.c,v
retrieving revision 1.334
diff -u -p -r1.334 gtktreeview.c
--- gtktreeview.c 15 Aug 2003 20:07:40 -0000 1.334
+++ gtktreeview.c 17 Aug 2003 01:53:26 -0000
@@ -40,6 +40,7 @@
#include "gtktreemodelsort.h"
#define GTK_TREE_VIEW_SEARCH_DIALOG_KEY "gtk-tree-view-search-dialog"
+
#define GTK_TREE_VIEW_PRIORITY_VALIDATE (GDK_PRIORITY_REDRAW + 5)
#define GTK_TREE_VIEW_PRIORITY_SCROLL_SYNC (GTK_TREE_VIEW_PRIORITY_VALIDATE + 2)
#define GTK_TREE_VIEW_NUM_ROWS_PER_IDLE 500
@@ -4784,26 +4785,67 @@ get_source_row (GdkDragContext *context)
return NULL;
}
+typedef struct
+{
+ GtkTreeRowReference *dest_row;
+ gboolean path_down_mode;
+}
+DestRow;
+
+static void
+dest_row_free (gpointer data)
+{
+ DestRow *dr = (DestRow *)data;
+
+ if (!dr)
+ return;
+
+ gtk_tree_row_reference_free (dr->dest_row);
+ g_free (dr);
+}
+
static void
set_dest_row (GdkDragContext *context,
GtkTreeModel *model,
- GtkTreePath *dest_row)
+ GtkTreePath *dest_row,
+ gboolean path_down_mode)
{
- g_object_set_data_full (G_OBJECT (context),
- "gtk-tree-view-dest-row",
- dest_row ? gtk_tree_row_reference_new (model, dest_row) : NULL,
- (GDestroyNotify) (dest_row ? gtk_tree_row_reference_free : NULL));
+ DestRow *dr;
+
+ if (!dest_row)
+ {
+ g_object_set_data_full (G_OBJECT (context), "gtk-tree-view-dest-row",
+ NULL, NULL);
+ return;
+ }
+
+ dr = g_new0 (DestRow, 1);
+
+ dr->dest_row = gtk_tree_row_reference_new (model, dest_row);
+ dr->path_down_mode = path_down_mode;
+
+ g_object_set_data_full (G_OBJECT (context), "gtk-tree-view-dest-row",
+ dr, (GDestroyNotify) dest_row_free);
}
static GtkTreePath*
-get_dest_row (GdkDragContext *context)
+get_dest_row (GdkDragContext *context,
+ gboolean *path_down_mode)
{
- GtkTreeRowReference *ref =
+ DestRow *dr =
g_object_get_data (G_OBJECT (context), "gtk-tree-view-dest-row");
- if (ref)
- return gtk_tree_row_reference_get_path (ref);
+ if (dr)
+ {
+ if (path_down_mode)
+ *path_down_mode = dr->path_down_mode;
+
+ if (dr->dest_row)
+ return gtk_tree_row_reference_get_path (dr->dest_row);
+ else
+ return NULL;
+ }
else
return NULL;
}
@@ -5047,6 +5089,7 @@ set_destination_row (GtkTreeView *tre
TreeViewDragInfo *di;
GtkWidget *widget;
GtkTreePath *old_dest_path = NULL;
+ gboolean can_drop = FALSE;
*suggested_action = 0;
*target = GDK_NONE;
@@ -5082,18 +5125,31 @@ set_destination_row (GtkTreeView *tre
&path,
&pos))
{
- /* can't drop here */
+ gint n_children;
+ GtkTreeModel *model;
+
remove_open_timeout (tree_view);
- gtk_tree_view_set_drag_dest_row (GTK_TREE_VIEW (widget),
- NULL,
- GTK_TREE_VIEW_DROP_BEFORE);
+ /* the row got dropped on empty space, let's setup a special case
+ */
if (path)
gtk_tree_path_free (path);
- /* don't propagate to parent though */
- return TRUE;
+ model = gtk_tree_view_get_model (tree_view);
+
+ tree_view->priv->empty_view_drop = 1;
+
+ n_children = gtk_tree_model_iter_n_children (model, NULL);
+ if (n_children)
+ path = gtk_tree_path_new_from_indices (n_children - 1, -1);
+ else
+ path = NULL;
+
+ pos = GTK_TREE_VIEW_DROP_AFTER;
+ can_drop = TRUE;
+
+ goto out;
}
g_assert (path);
@@ -5116,16 +5172,30 @@ set_destination_row (GtkTreeView *tre
if (TRUE /* FIXME if the location droppable predicate */)
{
+ can_drop = TRUE;
+ }
+ else
+ {
+ /* can't drop here */
+ remove_open_timeout (tree_view);
+
+ gtk_tree_view_set_drag_dest_row (GTK_TREE_VIEW (widget),
+ NULL,
+ GTK_TREE_VIEW_DROP_BEFORE);
+ }
+
+out:
+ if (can_drop)
+ {
GtkWidget *source_widget;
*suggested_action = context->suggested_action;
-
source_widget = gtk_drag_get_source_widget (context);
if (source_widget == widget)
{
/* Default to MOVE, unless the user has
- * pressed ctrl or alt to affect available actions
+ * pressed ctrl or shift to affect available actions
*/
if ((context->actions & GDK_ACTION_MOVE) != 0)
*suggested_action = GDK_ACTION_MOVE;
@@ -5134,29 +5204,24 @@ set_destination_row (GtkTreeView *tre
gtk_tree_view_set_drag_dest_row (GTK_TREE_VIEW (widget),
path, pos);
}
- else
- {
- /* can't drop here */
- remove_open_timeout (tree_view);
-
- gtk_tree_view_set_drag_dest_row (GTK_TREE_VIEW (widget),
- NULL,
- GTK_TREE_VIEW_DROP_BEFORE);
- }
if (path)
gtk_tree_path_free (path);
return TRUE;
}
-static GtkTreePath*
-get_logical_dest_row (GtkTreeView *tree_view)
+static GtkTreePath*
+get_logical_dest_row (GtkTreeView *tree_view,
+ gboolean *path_down_mode)
{
/* adjust path to point to the row the drop goes in front of */
GtkTreePath *path = NULL;
GtkTreeViewDropPosition pos;
+ if (path_down_mode)
+ *path_down_mode = FALSE;
+
gtk_tree_view_get_drag_dest_row (tree_view, &path, &pos);
if (path == NULL)
@@ -5167,8 +5232,8 @@ get_logical_dest_row (GtkTreeView *tree_
else if (pos == GTK_TREE_VIEW_DROP_INTO_OR_BEFORE ||
pos == GTK_TREE_VIEW_DROP_INTO_OR_AFTER)
{
- /* get first child, drop before it */
- gtk_tree_path_down (path);
+ if (path_down_mode)
+ *path_down_mode = TRUE;
}
else
{
@@ -5177,17 +5242,14 @@ get_logical_dest_row (GtkTreeView *tree_
g_assert (pos == GTK_TREE_VIEW_DROP_AFTER);
- gtk_tree_model_get_iter (model, &iter, path);
-
- if (!gtk_tree_model_iter_next (model, &iter))
- g_object_set_data (G_OBJECT (model), "gtk-tree-model-drop-append",
- GINT_TO_POINTER (1));
+ if (!gtk_tree_model_get_iter (model, &iter, path) ||
+ !gtk_tree_model_iter_next (model, &iter))
+ tree_view->priv->drop_append_mode = 1;
else
{
- g_object_set_data (G_OBJECT (model), "gtk-tree-model-drop-append",
- NULL);
- gtk_tree_path_next (path);
- }
+ tree_view->priv->drop_append_mode = 0;
+ gtk_tree_path_next (path);
+ }
}
return path;
@@ -5427,7 +5489,9 @@ gtk_tree_view_drag_motion (GtkWidget
gint y,
guint time)
{
+ gboolean empty;
GtkTreePath *path = NULL;
+ GtkTreeModel *model;
GtkTreeViewDropPosition pos;
GtkTreeView *tree_view;
GdkDragAction suggested_action = 0;
@@ -5440,7 +5504,11 @@ gtk_tree_view_drag_motion (GtkWidget
gtk_tree_view_get_drag_dest_row (tree_view, &path, &pos);
- if (path == NULL)
+ /* we only know this *after* set_desination_row */
+ model = gtk_tree_view_get_model (tree_view);
+ empty = tree_view->priv->empty_view_drop;
+
+ if (path == NULL && !empty)
{
/* Can't drop here. */
gdk_drag_status (context, 0, time);
@@ -5495,6 +5563,7 @@ gtk_tree_view_drag_drop (GtkWidget
GdkAtom target = GDK_NONE;
TreeViewDragInfo *di;
GtkTreeModel *model;
+ gboolean path_down_mode;
tree_view = GTK_TREE_VIEW (widget);
@@ -5514,7 +5583,7 @@ gtk_tree_view_drag_drop (GtkWidget
if (!set_destination_row (tree_view, context, x, y, &suggested_action, &target))
return FALSE;
- path = get_logical_dest_row (tree_view);
+ path = get_logical_dest_row (tree_view, &path_down_mode);
if (target != GDK_NONE && path != NULL)
{
@@ -5522,8 +5591,7 @@ gtk_tree_view_drag_drop (GtkWidget
* treat drag data receives as a drop.
*/
set_status_pending (context, 0);
-
- set_dest_row (context, model, path);
+ set_dest_row (context, model, path, path_down_mode);
}
if (path)
@@ -5559,6 +5627,7 @@ gtk_tree_view_drag_data_received (GtkWid
GtkTreeView *tree_view;
GtkTreePath *dest_row;
GdkDragAction suggested_action;
+ gboolean path_down_mode;
tree_view = GTK_TREE_VIEW (widget);
@@ -5581,17 +5650,32 @@ gtk_tree_view_drag_data_received (GtkWid
* supposed to call drag_status, not actually paste in the
* data.
*/
- path = get_logical_dest_row (tree_view);
+ path = get_logical_dest_row (tree_view, &path_down_mode);
if (path == NULL)
suggested_action = 0;
+ else if (path_down_mode)
+ gtk_tree_path_down (path);
if (suggested_action)
{
if (!gtk_tree_drag_dest_row_drop_possible (GTK_TREE_DRAG_DEST (model),
path,
selection_data))
- suggested_action = 0;
+ {
+ if (path_down_mode)
+ {
+ path_down_mode = FALSE;
+ gtk_tree_path_up (path);
+
+ if (!gtk_tree_drag_dest_row_drop_possible (GTK_TREE_DRAG_DEST (model),
+ path,
+ selection_data))
+ suggested_action = 0;
+ }
+ else
+ suggested_action = 0;
+ }
}
gdk_drag_status (context, suggested_action, time);
@@ -5608,10 +5692,34 @@ gtk_tree_view_drag_data_received (GtkWid
return;
}
- dest_row = get_dest_row (context);
+ dest_row = get_dest_row (context, &path_down_mode);
if (dest_row == NULL)
- return;
+ {
+ if (tree_view->priv->empty_view_drop)
+ dest_row = gtk_tree_path_new_from_indices (0, -1);
+ else
+ return;
+ }
+
+ if (tree_view->priv->drop_append_mode)
+ {
+ gtk_tree_path_next (dest_row);
+ tree_view->priv->drop_append_mode = 0;
+ }
+
+ if (selection_data->length >= 0)
+ {
+ if (path_down_mode)
+ {
+ gtk_tree_path_down (dest_row);
+ if (!gtk_tree_drag_dest_row_drop_possible (GTK_TREE_DRAG_DEST (model),
+ dest_row, selection_data))
+ gtk_tree_path_up (dest_row);
+ }
+ }
+
+ tree_view->priv->empty_view_drop = 0;
if (selection_data->length >= 0)
{
@@ -5637,7 +5745,7 @@ gtk_tree_view_drag_data_received (GtkWid
gtk_tree_path_free (dest_row);
/* drop dest_row */
- set_dest_row (context, NULL, NULL);
+ set_dest_row (context, NULL, NULL, FALSE);
}
@@ -10610,10 +10718,10 @@ gtk_tree_view_set_drag_dest_row (GtkTree
current_dest = NULL;
if (tree_view->priv->drag_dest_row)
- current_dest = gtk_tree_row_reference_get_path (tree_view->priv->drag_dest_row);
-
- if (tree_view->priv->drag_dest_row)
- gtk_tree_row_reference_free (tree_view->priv->drag_dest_row);
+ {
+ current_dest = gtk_tree_row_reference_get_path (tree_view->priv->drag_dest_row);
+ gtk_tree_row_reference_free (tree_view->priv->drag_dest_row);
+ }
tree_view->priv->drag_dest_pos = pos;
@@ -10660,7 +10768,12 @@ gtk_tree_view_get_drag_dest_row (GtkTree
if (tree_view->priv->drag_dest_row)
*path = gtk_tree_row_reference_get_path (tree_view->priv->drag_dest_row);
else
- *path = NULL;
+ {
+ if (tree_view->priv->empty_view_drop)
+ *path = gtk_tree_path_new_from_indices (0, -1);
+ else
+ *path = NULL;
+ }
}
if (pos)
Index: gtkliststore.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkliststore.c,v
retrieving revision 1.89
diff -u -p -r1.89 gtkliststore.c
--- gtkliststore.c 4 Jun 2003 23:53:35 -0000 1.89
+++ gtkliststore.c 17 Aug 2003 01:53:27 -0000
@@ -1497,32 +1497,22 @@ gtk_list_store_drag_data_received (GtkTr
/* dest was the first spot in the list; which means we are supposed
* to prepend.
*/
- gtk_list_store_prepend (GTK_LIST_STORE (tree_model),
- &dest_iter);
+ gtk_list_store_prepend (list_store, &dest_iter);
retval = TRUE;
}
else
{
- if (gtk_tree_model_get_iter (GTK_TREE_MODEL (tree_model),
- &dest_iter,
- prev))
+ if (gtk_tree_model_get_iter (tree_model, &dest_iter, prev))
{
GtkTreeIter tmp_iter = dest_iter;
- if (GPOINTER_TO_INT (g_object_get_data (G_OBJECT (tree_model), "gtk-tree-model-drop-append")))
- gtk_list_store_append (GTK_LIST_STORE (tree_model), &dest_iter);
- else
- gtk_list_store_insert_after (GTK_LIST_STORE (tree_model),
- &dest_iter, &tmp_iter);
+ gtk_list_store_insert_after (list_store, &dest_iter, &tmp_iter);
retval = TRUE;
}
}
- g_object_set_data (G_OBJECT (tree_model), "gtk-tree-model-drop-append",
- NULL);
-
gtk_tree_path_free (prev);
/* If we succeeded in creating dest_iter, copy data from src
@@ -1554,11 +1544,11 @@ gtk_list_store_drag_data_received (GtkTr
++col;
}
- dest_iter.stamp = GTK_LIST_STORE (tree_model)->stamp;
+ dest_iter.stamp = list_store->stamp;
G_SLIST (dest_iter.user_data)->data = copy_head;
- path = gtk_list_store_get_path (GTK_TREE_MODEL (tree_model), &dest_iter);
- gtk_tree_model_row_changed (GTK_TREE_MODEL (tree_model), path, &dest_iter);
+ path = gtk_list_store_get_path (tree_model, &dest_iter);
+ gtk_tree_model_row_changed (tree_model, path, &dest_iter);
gtk_tree_path_free (path);
}
}
@@ -1612,7 +1602,8 @@ gtk_list_store_row_drop_possible (GtkTre
retval = TRUE;
out:
- gtk_tree_path_free (src_path);
+ if (src_path)
+ gtk_tree_path_free (src_path);
return retval;
}
Index: gtktreestore.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtktreestore.c,v
retrieving revision 1.102
diff -u -p -r1.102 gtktreestore.c
--- gtktreestore.c 4 Jun 2003 23:53:35 -0000 1.102
+++ gtktreestore.c 17 Aug 2003 01:53:28 -0000
@@ -1754,7 +1754,7 @@ gtk_tree_store_drag_data_received (GtkTr
gtk_tree_path_free (parent);
parent = NULL;
- gtk_tree_store_prepend (GTK_TREE_STORE (tree_model),
+ gtk_tree_store_prepend (tree_store,
&dest_iter,
dest_parent_p);
@@ -1762,35 +1762,16 @@ gtk_tree_store_drag_data_received (GtkTr
}
else
{
- if (gtk_tree_model_get_iter (GTK_TREE_MODEL (tree_model),
- &dest_iter,
- prev))
+ if (gtk_tree_model_get_iter (tree_model, &dest_iter, prev))
{
GtkTreeIter tmp_iter = dest_iter;
- if (GPOINTER_TO_INT (g_object_get_data (G_OBJECT (tree_model), "gtk-tree-model-drop-append")))
- {
- GtkTreeIter parent;
+ gtk_tree_store_insert_after (tree_store, &dest_iter, NULL,
+ &tmp_iter);
- if (gtk_tree_model_iter_parent (GTK_TREE_MODEL (tree_model), &parent, &tmp_iter))
- gtk_tree_store_append (GTK_TREE_STORE (tree_model),
- &dest_iter, &parent);
- else
- gtk_tree_store_append (GTK_TREE_STORE (tree_model),
- &dest_iter, NULL);
- }
- else
- gtk_tree_store_insert_after (GTK_TREE_STORE (tree_model),
- &dest_iter,
- NULL,
- &tmp_iter);
retval = TRUE;
-
}
}
-
- g_object_set_data (G_OBJECT (tree_model), "gtk-tree-model-drop-append",
- NULL);
gtk_tree_path_free (prev);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
