Rework <<TreeviewSelect>>. Patch by pspjuth.

4 files changed, 96 insertions, 46 deletions

diff --git a/doc/text.n b/doc/text.n
index 76f6dfb..7498cb5 100644
--- a/doc/text.n
+++ b/doc/text.n
@@ -847,6 +847,9 @@ characters in the text.
 .IP [4]
 Whenever the \fBsel\fR tag range changes a virtual event \fB<<Selection>>\fR
 is generated.
+It might also be generated when selection is affected but not actually changed.
+Further, multiple selection changes could happen before events can be processed
+leading to multiple events with the same visible selection.
 .PP
 The \fBsel\fR tag is automatically defined when a text widget is created, and
 it may not be deleted with the
diff --git a/doc/ttk_treeview.n b/doc/ttk_treeview.n
index beb7690..29f066d 100644
--- a/doc/ttk_treeview.n
+++ b/doc/ttk_treeview.n
@@ -573,6 +573,9 @@ Again, \fBcolumn #0 always refers to the tree column\fR.
 The treeview widget generates the following virtual events.
 .IP <<TreeviewSelect>>
 Generated whenever the selection or cellselection changes.
+It might also be generated when selection is affected but not actually changed.
+Further, multiple selection changes could happen before events can be processed
+leading to multiple events with the same visible selection.
 .IP <<TreeviewOpen>>
 Generated just before setting the focus item to \fB\-open true\fR.
 .IP <<TreeviewClose>>
diff --git a/generic/ttk/ttkTreeview.c b/generic/ttk/ttkTreeview.c
index 4e8f9fe..d389bb9 100644
--- a/generic/ttk/ttkTreeview.c
+++ b/generic/ttk/ttkTreeview.c
@@ -570,18 +570,18 @@ static void foreachHashEntry(Tcl_HashTable *ht, HashEntryIterator func)
     }
 }
 
-static void CellSelectionClearCB(void *clientData)
+static int CellSelectionClear(Treeview *tv)
 {
-    TreeItem *item = (TreeItem *) clientData;
-    if (item->selObj != NULL) {
-	Tcl_DecrRefCount(item->selObj);
-	item->selObj = NULL;
+    TreeItem *item;
+    int anyChange = 0;
+    for (item=tv->tree.root; item; item = NextPreorder(item)) {
+	if (item->selObj != NULL) {
+	    Tcl_DecrRefCount(item->selObj);
+	    item->selObj = NULL;
+	    anyChange = 1;
+	}
     }
-}
-
-static void CellSelectionClear(Treeview *tv)
-{
-    foreachHashEntry(&tv->tree.items, CellSelectionClearCB);
+    return anyChange;
 }
 
 /* + unshareObj(objPtr) --
@@ -3272,9 +3272,15 @@ static int TreeviewDeleteCommand(
      */
     delq = 0;
     for (i = 0; items[i]; ++i) {
-        if (items[i]->state & TTK_STATE_SELECTED || items[i]->selObj != NULL) {
+        if (items[i]->state & TTK_STATE_SELECTED) {
             selChange = 1;
-        }
+        } else if (items[i]->selObj != NULL) {
+	    int length;
+	    Tcl_ListObjLength(interp, items[i]->selObj, &length);
+	    if (length > 0) {
+		selChange = 1;
+	    }
+	}
 	delq = DeleteItems(items[i], delq);
     }
 
@@ -3565,30 +3571,16 @@ static int TreeviewSelectionCommand(
     switch (selop)
     {
 	case SELECTION_SET:
+	    /* Clear */
 	    for (item=tv->tree.root; item; item = NextPreorder(item)) {
-		int inSetList = 0;
-
-		for (i=0; items[i]; ++i) {
-		    if (item == items[i]) {
-			inSetList = 1;
-			if (!(item->state & TTK_STATE_SELECTED)) {
-			    /* Item newly selected */
-			    selChange = 1;
-			}
-			break;
-		    }
-		}
-		if (!inSetList && (item->state & TTK_STATE_SELECTED)) {
-		    /* Item newly deselected */
+		if (item->state & TTK_STATE_SELECTED) {
+		    item->state &= ~TTK_STATE_SELECTED;
 		    selChange = 1;
 		}
-		if (selChange) break;
-	    }
-	    for (item=tv->tree.root; item; item = NextPreorder(item)) {
-		item->state &= ~TTK_STATE_SELECTED;
 	    }
 	    for (i=0; items[i]; ++i) {
 		items[i]->state |= TTK_STATE_SELECTED;
+		selChange = 1;
 	    }
 	    break;
 	case SELECTION_ADD:
@@ -3627,11 +3619,11 @@ static int TreeviewSelectionCommand(
 /* + SelObjChangeElement --
  * 	Change an element in a cell selection list.
  */
-static void SelObjChangeElement(
+static int SelObjChangeElement(
     Treeview *tv, Tcl_Obj *listPtr, Tcl_Obj *elemPtr,
     int add, TCL_UNUSED(int) /*remove*/, int toggle)
 {
-    int i, nElements;
+    int i, nElements, anyChange = 0;
     TreeColumn *column, *elemColumn;
     Tcl_Obj **elements;
 
@@ -3641,15 +3633,18 @@ static void SelObjChangeElement(
 	column = FindColumn(NULL, tv, elements[i]);
 	if (column == elemColumn) {
 	    if (add) {
-		return;
+		return anyChange;
 	    }
 	    Tcl_ListObjReplace(NULL, listPtr, i, 1, 0, NULL);
-	    return;
+	    anyChange = 1;
+	    return anyChange;
 	}
     }
     if (add || toggle) {
 	Tcl_ListObjAppendElement(NULL, listPtr, elemColumn->idObj);
+	anyChange = 1;
     }
+    return anyChange;
 }
 
 /* + $tree cellselection ?add|remove|set|toggle $items?
@@ -3661,7 +3656,7 @@ static int CellSelectionRange(
     TreeCell cellFrom, cellTo;
     TreeItem *item;
     Tcl_Obj *columns, **elements;
-    int colno, nElements, i, fromNo, toNo;
+    int colno, nElements, i, fromNo, toNo, anyChange = 0;
     int set = !(add || remove || toggle);
 
     if (GetCellFromObj(interp, tv, fromCell, 1, &fromNo, &cellFrom)
@@ -3694,7 +3689,7 @@ static int CellSelectionRange(
      * Start with clearing out.
      */
     if (set) {
-	CellSelectionClear(tv);
+	anyChange = CellSelectionClear(tv);
     }
 
     /* Correct order.
@@ -3716,7 +3711,7 @@ static int CellSelectionRange(
 
 	    Tcl_ListObjGetElements(NULL, columns, &nElements, &elements);
 	    for (i = 0; i < nElements; ++i) {
-		SelObjChangeElement(tv, item->selObj, elements[i],
+		anyChange |= SelObjChangeElement(tv, item->selObj, elements[i],
 			add, remove, toggle);
 	    }
 	} else {
@@ -3725,6 +3720,7 @@ static int CellSelectionRange(
 	    if (!remove) {
 		item->selObj = columns;
 		Tcl_IncrRefCount(item->selObj);
+		anyChange = 1;
 	    }
 	}
 	if (item == cellTo.item) {
@@ -3734,7 +3730,9 @@ static int CellSelectionRange(
 
     Tcl_DecrRefCount(columns);
 
-    Tk_SendVirtualEvent(tv->core.tkwin, "TreeviewSelect", NULL);
+    if (anyChange) {
+	Tk_SendVirtualEvent(tv->core.tkwin, "TreeviewSelect", NULL);
+    }
     TtkRedisplayWidget(&tv->core);
     return TCL_OK;
 }
@@ -3752,7 +3750,7 @@ static int TreeviewCellSelectionCommand(
     };
 
     Treeview *tv = (Treeview *)recordPtr;
-    int selop;
+    int selop, anyChange = 0;
     TkSizeT i, nCells;
     TreeCell *cells;
     TreeItem *item;
@@ -3812,7 +3810,7 @@ static int TreeviewCellSelectionCommand(
     switch (selop)
     {
 	case SELECTION_SET:
-	    CellSelectionClear(tv);
+	    anyChange = CellSelectionClear(tv);
 	    /*FALLTHRU*/
 	case SELECTION_ADD:
 	    for (i = 0; i < nCells; i++) {
@@ -3822,7 +3820,8 @@ static int TreeviewCellSelectionCommand(
 		    Tcl_IncrRefCount(item->selObj);
 		}
 		item->selObj = unshareObj(item->selObj);
-		SelObjChangeElement(tv, item->selObj, cells[i].colObj, 1, 0, 0);
+		anyChange |= SelObjChangeElement(tv, item->selObj,
+			cells[i].colObj, 1, 0, 0);
 	    }
 	    break;
 	case SELECTION_REMOVE:
@@ -3832,7 +3831,8 @@ static int TreeviewCellSelectionCommand(
 		    continue;
 		}
 		item->selObj = unshareObj(item->selObj);
-		SelObjChangeElement(tv, item->selObj, cells[i].colObj, 0, 1, 0);
+		anyChange |= SelObjChangeElement(tv, item->selObj,
+			cells[i].colObj, 0, 1, 0);
 	    }
 	    break;
 	case SELECTION_TOGGLE:
@@ -3843,13 +3843,16 @@ static int TreeviewCellSelectionCommand(
 		    Tcl_IncrRefCount(item->selObj);
 		}
 		item->selObj = unshareObj(item->selObj);
-		SelObjChangeElement(tv, item->selObj, cells[i].colObj, 0, 0, 1);
+		anyChange = SelObjChangeElement(tv, item->selObj,
+			cells[i].colObj, 0, 0, 1);
 	    }
 	    break;
     }
 
     ckfree(cells);
-    Tk_SendVirtualEvent(tv->core.tkwin, "TreeviewSelect", NULL);
+    if (anyChange) {
+	Tk_SendVirtualEvent(tv->core.tkwin, "TreeviewSelect", NULL);
+    }
     TtkRedisplayWidget(&tv->core);
 
     return TCL_OK;
diff --git a/tests/ttk/treeview.test b/tests/ttk/treeview.test
index 5864be1..f7168ee 100644
--- a/tests/ttk/treeview.test
+++ b/tests/ttk/treeview.test
@@ -525,8 +525,9 @@ test treeview-8.8 "<<TreeviewSelect>> when setting the selection" -body {
     set val 2
     .tv selection set myItem1  ; # <<TreeviewSelect>> triggers
     update
+    # Current implementation generates an event for this case
     set val 3
-    .tv selection set myItem1  ; # no <<TreeviewSelect>> (already selected)
+    .tv selection set myItem1  ; # (already selected)
     update
     set val 4
     .tv selection set {myItem1 myItem2}  ; # <<TreeviewSelect>> triggers
@@ -535,7 +536,9 @@ test treeview-8.8 "<<TreeviewSelect>> when setting the selection" -body {
     .tv selection set {myItem2}  ; # <<TreeviewSelect>> triggers
     update
     set res
-} -result {2 4 5}
+} -cleanup {
+    bind .tv <<TreeviewSelect>> {}
+} -result {2 3 4 5}
 
 test treeview-8.9 "<<TreeviewSelect>> when removing items from the selection" -body {
     .tv delete [.tv children {}]
@@ -554,6 +557,8 @@ test treeview-8.9 "<<TreeviewSelect>> when removing items from the selection" -b
     .tv selection remove myItem1  ; # no <<TreeviewSelect>> (selection unchanged)
     update
     set res
+} -cleanup {
+    bind .tv <<TreeviewSelect>> {}
 } -result {2}
 
 test treeview-8.10 "<<TreeviewSelect>> when adding items in the selection" -body {
@@ -574,6 +579,8 @@ test treeview-8.10 "<<TreeviewSelect>> when adding items in the selection" -body
     .tv selection add myItem3  ; # <<TreeviewSelect>> triggers
     update
     set res
+} -cleanup {
+    bind .tv <<TreeviewSelect>> {}
 } -result {1 3}
 
 test treeview-8.11 "<<TreeviewSelect>> when toggling" -body {
@@ -597,6 +604,8 @@ test treeview-8.11 "<<TreeviewSelect>> when toggling" -body {
     .tv selection toggle {myItem3 myItem2}  ; # <<TreeviewSelect>> triggers
     update
     set res
+} -cleanup {
+    bind .tv <<TreeviewSelect>> {}
 } -result {2 3 4}
 
 ### NEED: more tests for see/yview/scrolling
@@ -827,6 +836,8 @@ test treeview-11.12 "<<TreeviewSelect>> on selected cell deletion" -body {
     .tv insert "" end -id selectedDoomed -text DeadItem
     .tv insert "" end -id doomed -text AlsoDead
     .tv cellselection add {"selectedDoomed c"}
+    .tv cellselection add {"doomed c"}
+    .tv cellselection remove {"doomed c"}
     update
     bind .tv <<TreeviewSelect>> {lappend res 1}
     set res 0
@@ -857,6 +868,36 @@ test treeview-11.16 "Cellselection - non visible" -body {
     .tv cellselection
 } -result {{nn d}}
 
+# Same as 8.8, but for cell selection
+test treeview-11.17 "<<TreeviewSelect>> when setting the selection" -body {
+    .tv delete [.tv children {}]
+    .tv insert "" end -id myItem1 -text FirstItem
+    .tv insert "" end -id myItem2 -text SecondItem
+    update
+    bind .tv <<TreeviewSelect>> {lappend res $val}
+    set res {}
+    set val 1
+    .tv cellselection set ""   ; # no <<TreeviewSelect>> (selection unchanged)
+    update
+    set val 2
+    .tv cellselection set "myItem1 a"  ; # <<TreeviewSelect>> triggers
+    update
+    # Current implementation generates an event for this case
+    set val 3
+    .tv cellselection set "myItem1 a"  ; # (already selected)
+    update
+    set val 4
+    .tv cellselection set {{myItem1 a} {myItem2 a}} ; # <<TreeviewSelect>>
+    update
+    set val 5
+    .tv cellselection set {myItem2 a}  ; # <<TreeviewSelect>> triggers
+    update
+    set res
+} -cleanup {
+    bind .tv <<TreeviewSelect>> {}
+} -result {2 3 4 5}
+
+
 ### identify tests:
 #
 proc identify* {tv comps args} {