[cp-patches] RFA: more TextArea.java fixes

David Gilbert david.gilbert at object-refinery.com
Fri Jun 30 09:16:15 UTC 2006


Hi Tania,

Tania Bento wrote:

>Hey,
>
>This patch fixes a couple of Harmony's Tests that were failing on
>Classpath:
>
>- In the InsertText and AppendTextMethod, I added the case where peer ==
>null.
>- In the constructor, if the text specified by the user is null, it
>should be changed to "".
>
>Could someone please approve/comment on this patch so that I can commit
>it.
>
>Thanks,
>Tania
>
>2006-06-29  Tania Bento  <tbento at redhat.com>
>
>	*java/awt/TextArea.java
>	(TextArea): If text == null, change it to "".
>  
>
Your ChangeLog entry should identify the actual constructor:

(TextArea(String, int, int, int)): If text == null...

and I think you left out some of the changes (removed 
IllegalArgumentException and added new handling for negative rows and 
columns).

>	(InsertText): Added the case when peer == null.
>	(AppendText): Added the case when peer == null.
>  
>
The case is wrong for these method names in the ChangeLog entry.

>------------------------------------------------------------------------
>
>Index: java/awt/TextArea.java
>===================================================================
>RCS file: /cvsroot/classpath/classpath/java/awt/TextArea.java,v
>retrieving revision 1.20
>diff -u -r1.20 TextArea.java
>--- java/awt/TextArea.java	20 Sep 2005 01:05:28 -0000	1.20
>+++ java/awt/TextArea.java	29 Jun 2006 14:34:04 -0000
>@@ -187,23 +187,25 @@
>    */
>   public TextArea (String text, int rows, int columns, int scrollbarVisibility)
>   {
>-    super (text);
>+    super ((text == null) ? "" : text); 
>  
>
My guess is that this can and should be handled by the superclass 
(TextComponent) since the same problem occurs for the TextField class 
(which is also a subclass of TextComponent) - I added Mauve tests for 
the constructors for both classes.  You should also update the API docs 
for all the constructors that accept a String argument to note how a 
null value is handled.

I didn't look at the other part of your patch (insertText, appendText).  
If you have any doubts, write some Mauve tests...

Regards,

Dave



More information about the Classpath-patches mailing list