[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [cp-patches] java.awt.Choice selection behaviour when when no peer i
From: |
Noa Resare |
Subject: |
Re: [cp-patches] java.awt.Choice selection behaviour when when no peer is set |
Date: |
Mon, 25 Oct 2004 11:13:08 +0200 |
sön 2004-10-24 klockan 10:31 +0200 skrev Mark Wielaard:
> Hi,
>
> On Sun, 2004-10-24 at 00:24, Noa Resare wrote:
> > If you just instantiate a java.awt.Choice object and add/remove items
> > the rules for what item gets selected isn't obeyed since there is no
> > peer widget set instantiated. The attached patch implements this
> > behavior, so that the mauve testcases work.
>
> Thanks. Don't forget to update the copyright year in the files you edit.
> And in general we try to list all methods that changed explicitly like:
>
> 2004-10-22 Noa Resare <address@hidden>
>
> * java/awt/Choice.java (add):
> Implement correct selection behavior when peer == null.
> (insert): Likewise.
> (remove): Likewise.
>
> And the entries should be full sentences, ending with a dot. See also:
> "Documenting what changed when with ChangeLog entries"
> http://www.gnu.org/software/classpath/docs/hacking.html#SEC9
Ok. I'll try to stick to that in the future. Thanks for the friendly
correction.
> > The code I have added only affects the no-peer case, so any errors in it
> > shouldn't effect anyone doing anything real :)
>
> Which I actually think is a flaw in your patch.
> The tests in mauve were added since some programs first set up a Choice,
> fill it with items and/or manipulate it somehow before it gets shown.
> So I think something like the following (untested) needs to be added:
>
> * gnu/java/awt/peer/gtk/GtkChoicePeer.java (GtkChoicePeer):
> Call select() when Choice has a selected item.
I have tested your code with the attached very simple case, and it seems
to work like expected.
> > + if (index == selectedIndex)
> > + select(0);
> > + if (getItemCount() == 0)
> > + selectedIndex = -1;
>
> I think this should be:
>
> if (getItemCount() == 0)
> selectedIndex = -1;
> else if (index == selectedIndex)
> select(0);
>
> Otherwise you will get a IllegalArgumentException from select(0) if you
> remove the last item.
>
> What do you think?
Actually, you get away without an IllegalArgumentException as it seems
like select() implements a literal interpretation of the specification
("IllegalArgumentException - if the specified position is greater than
the number of items or less than zero") which is not taking into account
that position is 0-based and 'number of items' is 1-based, so it is
actually legal to place the position right after the last item.
Of course your solution is better.
cheers/noa
ChoiceTester.java
Description: Text Data