Tania's email is currently broken but after talking to her, this patch
is now commited.
2006-07-28 Anthony Balkissoon <[EMAIL PROTECTED]>
* java/awt/List.java: Committed following patch from Tania.
Initialized private variable visibleIndex to -1.
(addItem(String, int)): If string is null, set to empty string. If int
< -1, set to -1.
(delItem): If the item to be deleted was selected, select the next
item in the list.
(delItems): Checks are not necessary and all indices in selected should
be deleted.
(select): Update selected.
(deselect): Update selected.
--Tony
On Thu, 2006-07-27 at 17:55 -0400, Anthony Balkissoon wrote:
> On Thu, 2006-07-27 at 17:05 -0400, Anthony Balkissoon wrote:
> > A couple of things to check first:
> >
> > - getSelectedIndex() should probably return -1 if no items are selected,
> > not 0.
> Correct, it should return -1, the patch that Tania submitted did return
> -1, the branch "if (selected == null) return 0" was never taken because
> selected was initialized in the constructor and not set to null
> anywhere. It could potentially be null if the peer returns null for
> getSelectedIndexes() and in this case -1 should still be returned so I
> removed that if statement.
>
> > - in delItem, if the last item is deleted, but it was selected, it looks
> > like an ArrayIndexOutOfBoundsException will be thrown
> Actually, no. If there are 3 items and the third one (index 2) is
> selected, and then delItem(2) is called, isSelected(2) still returns
> true. This doesn't throw any exceptions. getItem(2) will throw an
> exception but isSelected(2) should still return true. I augmented the
> Mauve tests to show this.
>
> > - what happens in select() if multiple mode is false and another index
> > is already selected? should be call deselect() here?
> The way it is now seems to be fine, initializing a new int array.
>
>
> All of these things should also be tested when there IS a peer present.
> For instance, in select(), if a peer is present, we call peer.select().
> I wonder if we should return at this point or continue on with the
> method. These tests need to be written.
>
>
> The patch attached is essentially Tania's patch, with the aforementioned
> removal of an if statement and also with one small performance tweak in
> the select() method when multipleMode is false.
>
> I'll leave this for Tania to comment on and commit.
>
> --Tony
Index: java/awt/List.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/List.java,v
retrieving revision 1.28
diff -u -r1.28 List.java
--- java/awt/List.java 13 Jul 2006 17:30:24 -0000 1.28
+++ java/awt/List.java 28 Jul 2006 15:13:38 -0000
@@ -108,7 +108,7 @@
* @serial An index value used by <code>makeVisible()</code> and
* <code>getVisibleIndex</code>.
*/
-private int visibleIndex;
+private int visibleIndex = -1;
// The list of ItemListeners for this object.
private ItemListener item_listeners;
@@ -116,7 +116,6 @@
// The list of ActionListeners for this object.
private ActionListener action_listeners;
-
/*************************************************************************/
/*
@@ -176,6 +175,7 @@
if (GraphicsEnvironment.isHeadless())
throw new HeadlessException ();
+
}
/*************************************************************************/
@@ -314,12 +314,13 @@
*/
public void
setMultipleSelections(boolean multipleMode)
-{
+{
this.multipleMode = multipleMode;
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
peer.setMultipleMode (multipleMode);
+
}
/*************************************************************************/
@@ -519,6 +520,12 @@
public void
addItem(String item, int index)
{
+ if (item == null)
+ item = "";
+
+ if (index < -1)
+ index = -1;
+
if ((index == -1) || (index >= items.size ()))
items.addElement (item);
else
@@ -543,7 +550,17 @@
public void
delItem(int index) throws IllegalArgumentException
{
+ boolean selected = false;
+ if (isSelected(index))
+ {
+ selected = true;
+ deselect(index);
+ }
+
items.removeElementAt (index);
+
+ if (selected)
+ select(index);
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
@@ -580,15 +597,6 @@
public synchronized void
delItems(int start, int end) throws IllegalArgumentException
{
- if ((start < 0) || (start >= items.size()))
- throw new IllegalArgumentException("Bad list start index value: " + start);
-
- if ((start < 0) || (start >= items.size()))
- throw new IllegalArgumentException("Bad list start index value: " + start);
-
- if (start > end)
- throw new IllegalArgumentException("Start is greater than end!");
-
// We must run the loop in reverse direction.
for (int i = end; i >= start; --i)
items.removeElementAt (i);
@@ -644,6 +652,8 @@
ListPeer peer = (ListPeer) getPeer ();
if (peer != null)
peer.removeAll ();
+
+ selected = new int[0];
}
/*************************************************************************/
@@ -695,6 +705,7 @@
if (selected == null || selected.length != 1)
return -1;
+
return selected[0];
}
@@ -713,7 +724,8 @@
{
ListPeer l = (ListPeer) peer;
selected = l.getSelectedIndexes ();
- }
+ }
+
return selected;
}
@@ -862,13 +874,32 @@
*
* @param index The index of the item to select.
*/
-public synchronized void
-select(int index)
-{
- ListPeer lp = (ListPeer)getPeer();
- if (lp != null)
- lp.select(index);
-}
+ public synchronized void select(int index)
+ {
+ ListPeer lp = (ListPeer) getPeer();
+ if (lp != null)
+ lp.select(index);
+
+ boolean found = false;
+ for (int i = 0; i < selected.length; i++)
+ {
+ if (selected[i] == index)
+ found = true;
+ }
+ if (! found)
+ {
+ if (! isMultipleMode())
+ {
+ selected = new int[] { index };
+ return;
+ }
+
+ int[] temp = new int[selected.length + 1];
+ System.arraycopy(selected, 0, temp, 0, selected.length);
+ temp[selected.length] = index;
+ selected = temp;
+ }
+ }
/*************************************************************************/
@@ -880,9 +911,26 @@
public synchronized void
deselect(int index)
{
- ListPeer lp = (ListPeer)getPeer();
- if (lp != null)
- lp.deselect(index);
+ if (isSelected(index))
+ {
+ ListPeer lp = (ListPeer)getPeer();
+ if (lp != null)
+ lp.deselect(index);
+
+ int[] temp = new int[selected.length - 1];
+ for (int i = 0; i < temp.length; i++)
+ {
+ if (selected[i] != index)
+ temp[i] = selected[i];
+ else
+ {
+ System.arraycopy(selected, i + 1, temp, i,
+ selected.length - i - 1);
+ break;
+ }
+ }
+ selected = temp;
+ }
}
/*************************************************************************/
