[cp-patches] Re: [gui] Some (random) AWT and Swing fixlets

[Mark Wielaard]

Hi,

On Sun, 2005-02-13 at 15:53 -0500, Thomas Fitzsimmons wrote:
> > -   * @return the specified keystroke
> > -   * @throws NullPointerException if s is null
> > -   * @throws IllegalArgumentException if s cannot be parsed
> > +   * @return the specified keystroke, or null when s is null
> > +   * or cannot be parsed correctly.
> 
> The javadocs disagree with this change.

Ah, I see why I was confused. javax.swing.KeyStroke.getKeyStroke() calls
getAWTKeyStroke() but behaves a little different when the String cannot
be parsed. I changed getAWTKeyStroke() it now throws an
IllegalArgumentException (plus explanation) when it cannot parse the
keystroke declaration. And changed KeyStroke.getKeyStroke() to return
null if an IllegalArgumentException() would be thrown. (FreeMind uses
the illegal keystoke description "none" when it doesn't want to set a
specific key and expects javax.swing.KeyStroke.getKeyStroke() to return
null in that case).

> > +  /**
> > +   * Returns the viewSize when set, or the size of the set Component
> > view.
> > +   * If no viewSize and no Component view is set an empty Dimension
> > is
> > +   * returned.
> > +   */
> >    public Dimension getViewSize()
> >    {
> >      if (isViewSizeSet)
> >        return viewSize;
> >      else
> > -      return getView().getSize();
> > +      {
> > +       Component view = getView();
> > +       if (view != null)
> > +         return view.getSize();
> 
> According to the javadocs, you should be returning the preferred size
> here.

Yes, that makes sense.

> The other changes are fine.
Thanks for the review.

This is the patch as I have committed it:

2005-02-13  Mark Wielaard  <address@hidden>

        * java/awt/AWTKeyStroke.java (getAWTKeyStroke(String)): Throw
        IllegalArgumentException when the given String is null.

        * javax/swing/KeyStroke.java (getKeyStroke(String)): Return null
        when given keystoke sequence cannot be parsed.

        * javax/swing/JRootPane.java (setJMenuBar): Remove current menubar
        if one is installed. Only install the given menubar is not null.

        * javax/swing/JViewport.java (getViewSize): Return an empty
        Dimension when the view isn't set or preferred component size when
        no viewSize is set.

        * javax/swing/ViewportLayout.java (preferredLayoutSize): Return an
        empty Dimension when there is no view set.
        (minimumLayoutSize): Likewise.
        (layoutContainer): Don't try to layout when there is no view.

Cheers,

Mark

Index: java/awt/AWTKeyStroke.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/AWTKeyStroke.java,v
retrieving revision 1.6
diff -u -r1.6 AWTKeyStroke.java
--- java/awt/AWTKeyStroke.java  27 Sep 2004 15:11:46 -0000      1.6
+++ java/awt/AWTKeyStroke.java  13 Feb 2005 23:30:53 -0000
@@ -1,5 +1,5 @@
 /* AWTKeyStroke.java -- an immutable key stroke
-   Copyright (C) 2002, 2004  Free Software Foundation
+   Copyright (C) 2002, 2004, 2005  Free Software Foundation
 
 This file is part of GNU Classpath.
 
@@ -393,15 +393,16 @@
    * </code>      
    *
    * @param s the string to parse
+   * @throws IllegalArgumentException if s is null or cannot be parsed
    * @return the specified keystroke
-   * @throws NullPointerException if s is null
-   * @throws IllegalArgumentException if s cannot be parsed
    */
   public static AWTKeyStroke getAWTKeyStroke(String s)
   {
+    if (s == null)
+      throw new IllegalArgumentException("null argument");
     StringTokenizer t = new StringTokenizer(s, " ");
     if (! t.hasMoreTokens())
-      throw new IllegalArgumentException();
+      throw new IllegalArgumentException("no tokens '" + s + "'");
     int modifiers = 0;
     boolean released = false;
     String token = null;
@@ -432,7 +433,8 @@
                                          KeyEvent.VK_UNDEFINED, modifiers,
                                          false);
               }
-            throw new IllegalArgumentException();
+            throw new IllegalArgumentException("Invalid 'typed' argument '"
+                                              + s + "'");
           }
         else if ("pressed".equals(token))
           {
@@ -453,8 +455,11 @@
     while (t.hasMoreTokens());
     // Now token contains the VK name we must parse.
     Integer code = (Integer) vktable.get(token);
-    if (code == null || t.hasMoreTokens())
-      throw new IllegalArgumentException();
+    if (code == null)
+      throw new IllegalArgumentException("Unknown token '" + token
+                                        + "' in '" + s + "'");
+    if (t.hasMoreTokens())
+      throw new IllegalArgumentException("Too many tokens: " + s);
     return getAWTKeyStroke(KeyEvent.CHAR_UNDEFINED, code.intValue(),
                            modifiers, released);
   }
Index: javax/swing/KeyStroke.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/KeyStroke.java,v
retrieving revision 1.13
diff -u -r1.13 KeyStroke.java
--- javax/swing/KeyStroke.java  26 Jan 2005 20:57:45 -0000      1.13
+++ javax/swing/KeyStroke.java  13 Feb 2005 23:30:53 -0000
@@ -96,9 +96,22 @@
     return (KeyStroke) getAWTKeyStroke(keyCode, modifiers);
   }
 
+  /**
+   * Returns the KeyStroke according to <code>getAWTKeyStroke()</code>.
+   * But it returns null instead of throwing
+   * <code>IllegalArugmentException</code> when
+   * the keystoke sequence cannot be parsed from the given string.
+   */
   public static KeyStroke getKeyStroke(String str) 
   {
-    return (KeyStroke) getAWTKeyStroke(str);
+    try
+      {
+       return (KeyStroke) getAWTKeyStroke(str);
+      }
+    catch (IllegalArgumentException iae)
+      {
+       return null;
+      }
   }
 
   public static KeyStroke getKeyStrokeForEvent(KeyEvent event) 
Index: javax/swing/JRootPane.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/JRootPane.java,v
retrieving revision 1.19
diff -u -r1.19 JRootPane.java
--- javax/swing/JRootPane.java  31 Dec 2004 10:19:44 -0000      1.19
+++ javax/swing/JRootPane.java  13 Feb 2005 23:30:53 -0000
@@ -346,8 +346,12 @@
    */
   public void setJMenuBar(JMenuBar m)
   {
+    JLayeredPane jlPane = getLayeredPane();
+    if (menuBar != null)
+      jlPane.remove(menuBar);
     menuBar = m;
-    getLayeredPane().add(menuBar, JLayeredPane.FRAME_CONTENT_LAYER);
+    if (menuBar != null)
+      jlPane.add(menuBar, JLayeredPane.FRAME_CONTENT_LAYER);
   }
 
   /**
Index: javax/swing/JViewport.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/JViewport.java,v
retrieving revision 1.16
diff -u -r1.16 JViewport.java
--- javax/swing/JViewport.java  25 Jan 2005 07:07:25 -0000      1.16
+++ javax/swing/JViewport.java  13 Feb 2005 23:30:54 -0000
@@ -158,12 +158,23 @@
     fireStateChanged();
   }
 
+  /**
+   * Returns the viewSize when set, or the preferred size of the set
+   * Component view.  If no viewSize and no Component view is set an
+   * empty Dimension is returned.
+   */
   public Dimension getViewSize()
   {
     if (isViewSizeSet)
       return viewSize;
     else
-      return getView().getSize();
+      {
+       Component view = getView();
+       if (view != null)
+         return view.getPreferredSize();
+       else
+         return new Dimension();
+      }
   }
 
 
Index: javax/swing/ViewportLayout.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/ViewportLayout.java,v
retrieving revision 1.11
diff -u -r1.11 ViewportLayout.java
--- javax/swing/ViewportLayout.java     27 Dec 2004 11:24:47 -0000      1.11
+++ javax/swing/ViewportLayout.java     13 Feb 2005 23:30:54 -0000
@@ -63,17 +63,25 @@
   public void removeLayoutComponent(Component c) 
   {
   }
+
   public Dimension preferredLayoutSize(Container parent) 
   {
     JViewport vp = (JViewport)parent;
     Component view = vp.getView();
-    return view.getPreferredSize();
+    if (view != null)
+      return view.getPreferredSize();
+    else
+      return new Dimension();
   }
+
   public Dimension minimumLayoutSize(Container parent) 
   {
     JViewport vp = (JViewport)parent;
     Component view = vp.getView();
-    return view.getMinimumSize();
+    if (view != null)
+      return view.getMinimumSize();
+    else
+      return new Dimension();
   }
 
   /**
@@ -113,6 +121,9 @@
     JViewport port = (JViewport) parent;    
     Component view = port.getView();
 
+    if (view == null)
+      return;
+
     // These dimensions and positions are in *view space*.  Do not mix
     // variables in here from port space (eg. parent.getBounds()). This
     // function should be entirely in view space, because the methods on

signature.asc
Description: This is a digitally signed message part