[cp-patches] FYI: Adding read/write to javax/swing/text/JTextComponent.java (fixing bug #13640)
This should close the recently submitted bug #13640. These two missing methods are not complicated. As the field 'doc' is not directly assigned, I added the check for null. We do not know how the user may override setText. 2005-07-05 Audrius Meskauskas <[EMAIL PROTECTED]> * javax/swing/text/JTextComponent.java (read, write): New methods. Index: javax/swing/text/JTextComponent.java === RCS file: /cvsroot/classpath/classpath/javax/swing/text/JTextComponent.java,v retrieving revision 1.32 diff -u -r1.32 JTextComponent.java --- javax/swing/text/JTextComponent.java2 Jul 2005 20:32:51 - 1.32 +++ javax/swing/text/JTextComponent.java5 Jul 2005 07:28:00 - @@ -54,6 +54,8 @@ import java.awt.event.InputMethodListener; import java.awt.event.KeyEvent; import java.io.IOException; +import java.io.Reader; +import java.io.Writer; import java.util.Enumeration; import java.util.Hashtable; @@ -1611,4 +1613,55 @@ { navigationFilter = filter; } + + /** + * Read and set the content this component. If not overridden, the + * method reads the component content as a plain text. + * + * The second parameter of this method describes the input stream. It can + * be String, URL, File and so on. If not null, this object is added to + * the properties of the associated document under the key + * [EMAIL PROTECTED] Document#StreamDescriptionProperty}. + * + * @param input an input stream to read from. + * @param streamDescription an object, describing the stream. + * + * @throws IOException if the reader throws it. + * + * @see getDocument() + * @see Document#getProperty(Object) + */ + public void read(Reader input, Object streamDescription) +throws IOException + { +if (streamDescription != null) + { +Document d = getDocument(); +if (d != null) + d.putProperty(Document.StreamDescriptionProperty, streamDescription); + } + +StringBuffer b = new StringBuffer(); +int c; + +// Read till -1 (EOF). +while ((c = input.read()) >= 0) + b.append((char) c); + +setText(b.toString()); + } + + /** + * Write the content of this component to the given stream. If not + * overridden, the method writes the component content as a plain text. + * + * @param output the writer to write into. + * + * @throws IOException if the writer throws it. + */ + public void write(Writer output) + throws IOException + { +output.write(getText()); + } } ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: fixes for JTable API docs
I committed this patch to fix a lot of Eclipse warnings about the API docs for JTable: 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * javax/swing/JTable.java: fixed API doc links all over. Regards, Dave Gilbert ? diff.txt Index: javax/swing/JTable.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JTable.java,v retrieving revision 1.29 diff -u -r1.29 JTable.java --- javax/swing/JTable.java 2 Jul 2005 20:32:49 - 1.29 +++ javax/swing/JTable.java 5 Jul 2005 08:57:35 - @@ -356,10 +356,10 @@ * [EMAIL PROTECTED] TableColumnModel} and assign it to the [EMAIL PROTECTED] #columnModel} * property when the [EMAIL PROTECTED] #dataModel} property is changed. * - * @see #setModel() + * @see #setModel(TableModel) * @see #createColumnsFromModel() - * @see #setColumnModel() - * @see #setAutoCreateColumnsFromModel() + * @see #setColumnModel(TableColumnModel) + * @see #setAutoCreateColumnsFromModel(boolean) * @see #getAutoCreateColumnsFromModel() */ protected boolean autoCreateColumnsFromModel; @@ -371,7 +371,7 @@ * #AUTO_RESIZE_SUBSEQUENT_COLUMNS}, or [EMAIL PROTECTED] #AUTO_RESIZE_OFF}. * * @see #doLayout() - * @see #setAutoResizeMode() + * @see #setAutoResizeMode(int) * @see #getAutoResizeMode() */ protected int autoResizeMode; @@ -383,21 +383,21 @@ * [EMAIL PROTECTED] #columnModel}. * * @see #getRowHeight() - * @see #setRowHeight() + * @see #setRowHeight(int) * @see TableColumn#getWidth() - * @see TableColumn#setWidth() + * @see TableColumn#setWidth(int) */ protected int rowHeight; /** * The height in pixels of the gap left between any two rows of the table. * - * @see #setRowMargin() + * @see #setRowMargin(int) * @see #getRowHeight() * @see #getIntercellSpacing() - * @see #setIntercellSpacing() + * @see #setIntercellSpacing(Dimension) * @see TableColumnModel#getColumnMargin() - * @see TableColumnModel#setColumnMargin() + * @see TableColumnModel#setColumnMargin(int) */ protected int rowMargin; @@ -408,18 +408,18 @@ * selection as an independent concept, but it is now represented solely * in terms of simultaneous row and column selection. * - * @see TableColumnModel#columnSelectionAllowed() - * @see #setRowSelectionAllowed() + * @see TableColumnModel#getColumnSelectionAllowed() + * @see #setRowSelectionAllowed(boolean) * @see #getRowSelectionAllowed() * @see #getCellSelectionEnabled() - * @see #setCellSelectionEnabled() + * @see #setCellSelectionEnabled(boolean) */ protected boolean rowSelectionAllowed; /** - * @deprecated Use [EMAIL PROTECTED] #rowSelectionAllowed}, [EMAIL PROTECTED] - * #columnSelectionAllowed}, or the combined methods [EMAIL PROTECTED] - * getCellSelectionEnabled} and [EMAIL PROTECTED] setCellSelectionEnabled}. + * @deprecated Use [EMAIL PROTECTED] #rowSelectionAllowed}, [EMAIL PROTECTED] + * #getColumnSelectionAllowed}, or the combined methods [EMAIL PROTECTED] + * #getCellSelectionEnabled} and [EMAIL PROTECTED] #setCellSelectionEnabled(boolean)}. */ protected boolean cellSelectionEnabled; @@ -429,8 +429,8 @@ * property name. The table listens to its model as a [EMAIL PROTECTED] * TableModelListener}. * - * @see #tableChanged() - * @see TableModel#addTableModelListener() + * @see #tableChanged(TableModelEvent) + * @see TableModel#addTableModelListener(TableModelListener) */ protected TableModel dataModel; @@ -444,7 +444,7 @@ * * The TableColumnModel also contains a [EMAIL PROTECTED] ListSelectionModel} which * indicates which columns are currently selected. This selection model - * works in combination with the [EMAIL PROTECTED] selectionModel} of the table + * works in combination with the [EMAIL PROTECTED] #selectionModel} of the table * itself to specify a table selection: a combination of row and * column selections. * @@ -453,7 +453,7 @@ * columnModel automatically, and the table acts as a facade for most of * the interesting properties of the columnModel anyways. * - * @see #setColumnModel() + * @see #setColumnModel(TableColumnModel) * @see #getColumnModel() */ protected TableColumnModel columnModel; @@ -461,14 +461,14 @@ /** * A model of the rows of this table which are currently selected. This * model is used in combination with the column selection model held as a - * member of the [EMAIL PROTECTED] columnModel} property, to represent the rows and + * member of the [EMAIL PROTECTED] #columnModel} property, to represent the rows and * columns (or both: cells) of the table which are currently selected. * * @see #rowSelectionAllowed - * @see #setSelectionModel() + * @see #setSelectionModel(ListSelectionModel) * @see #getSelectionModel() * @see Ta
[cp-patches] FYI: minor API doc fixes in java.util.*
I committed this patch to fix some minor problems in the API docs for the java.util.* package: 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * java/util/BitSet.java: fixed minor problems in API docs, * java/util/Calendar.java: likewise, * java/util/Collections.java: likewise, * java/util/Date.java: likewise, * java/util/Hashtable.java: likewise, * java/util/LinkedList.java: likewise, * java/util/Properties.java: likewise, * java/util/PropertyResourceBundle.java: likewise, * java/util/ResourceBundle.java: likewise, * java/util/SimpleTimeZone.java: likewise, * java/util/TreeMap.java: likewise. Regards, Dave Gilbert Index: java/util/BitSet.java === RCS file: /cvsroot/classpath/classpath/java/util/BitSet.java,v retrieving revision 1.18 diff -u -r1.18 BitSet.java --- java/util/BitSet.java 2 Jul 2005 20:32:41 - 1.18 +++ java/util/BitSet.java 5 Jul 2005 10:14:24 - @@ -116,8 +116,8 @@ * given set. This means it builds the intersection * of the two sets. The result is stored into this bit set. * - * @param set the second bit set - * @throws NullPointerException if set is null + * @param bs the second bit set + * @throws NullPointerException if bs is null */ public void and(BitSet bs) { @@ -131,13 +131,13 @@ /** * Performs the logical AND operation on this bit set and the - * complement of the given set. This means it + * complement of the given bs. This means it * selects every element in the first set, that isn't in the * second set. The result is stored into this bit set and is * effectively the set difference of the two. * - * @param set the second bit set - * @throws NullPointerException if set is null + * @param bs the second bit set + * @throws NullPointerException if bs is null * @since 1.2 */ public void andNot(BitSet bs) @@ -190,12 +190,12 @@ } /** - * Removes the integer bitIndex from this set. That is + * Removes the integer pos from this set. That is * the corresponding bit is cleared. If the index is not in the set, * this method does nothing. * - * @param bitIndex a non-negative integer - * @throws IndexOutOfBoundsException if bitIndex < 0 + * @param pos a non-negative integer + * @throws IndexOutOfBoundsException if pos < 0 */ public void clear(int pos) { @@ -336,8 +336,8 @@ * set, otherwise false. * * @param pos a non-negative integer - * @return the value of the bit at the specified index - * @throws IndexOutOfBoundsException if the index is negative + * @return the value of the bit at the specified position + * @throws IndexOutOfBoundsException if the pos is negative */ public boolean get(int pos) { Index: java/util/Calendar.java === RCS file: /cvsroot/classpath/classpath/java/util/Calendar.java,v retrieving revision 1.45 diff -u -r1.45 Calendar.java --- java/util/Calendar.java 2 Jul 2005 20:32:41 - 1.45 +++ java/util/Calendar.java 5 Jul 2005 10:14:25 - @@ -897,7 +897,6 @@ /** * Fills any unset fields in the time field list - * @return true if the specified field has a value. */ protected void complete() { Index: java/util/Collections.java === RCS file: /cvsroot/classpath/classpath/java/util/Collections.java,v retrieving revision 1.36 diff -u -r1.36 Collections.java --- java/util/Collections.java 2 Jul 2005 20:32:41 - 1.36 +++ java/util/Collections.java 5 Jul 2005 10:14:32 - @@ -298,7 +298,7 @@ * * @param index The index of the element to retrieve. * @return the object at the specified index. - * @throws IndexOutofBoundsException as any given index + * @throws IndexOutOfBoundsException as any given index * is outside the bounds of an empty array. */ public Object get(int index) @@ -2137,7 +2137,7 @@ * within the underlying collection, first obtaining * a lock on the mutex. * - * @param cl the collection to test for. + * @param c1 the collection to test for. * @return true if for every element o in c, contains(o) * would return true. * @throws ClassCastException if the type of any element in cl is not a valid @@ -3396,7 +3396,7 @@ * operation). If the map already contains a key, its value is replaced. * A lock is obtained on the mutex before the operation proceeds. * - * @param m the mapping to load into this map + * @param map the mapping to load into this map * @throws UnsupportedOperationException if the operation is not supported * @throws ClassCastException if a key or value is of the wrong type * @throws IllegalArgumentException i
[cp-patches] Revised patch: JTable and DefaultTableColumnModel
Attached is a revised version of the patch I proposed late last week. It adds implementations for the isCellEditable() and createDefaultColumnsFromModel() methods in JTable, and fixes some bugs uncovered while writing Mauve tests for the new methods. 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * javax/swing/JTable.java: (addColumn): retrieve correct column name, (convertColumnIndexToModel): remove check for > columnCount and let exception happen, (getColumnName): retrieve name from TableColumn, (isCellEditable): implemented, (createDefaultColumnsFromModel): implemented. * javax/swing/table/DefaultTableColumnModel.java: (addColumn): throw exception for null argument, set correct column index in TableColumnModelEvent, (removeColumn): use correct column index, (moveColumn): move the column, don't swap it. Also added argument checks, (getColumnIndex): reimplemented. I've committed more Mauve tests to cover these changes. OK to commit? Regards, Dave Gilbert Index: javax/swing/JTable.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JTable.java,v retrieving revision 1.30 diff -u -r1.30 JTable.java --- javax/swing/JTable.java 5 Jul 2005 09:03:43 - 1.30 +++ javax/swing/JTable.java 5 Jul 2005 11:11:06 - @@ -672,8 +672,8 @@ { if (column.getHeaderValue() == null) { - String name = getColumnName(column.getModelIndex()); - column.setHeaderValue(name); +String name = dataModel.getColumnName(column.getModelIndex()); +column.setHeaderValue(name); } columnModel.addColumn(column); @@ -1025,8 +1025,6 @@ { if (vc < 0) return vc; -else if (vc > getColumnCount()) - return -1; else return columnModel.getColumn(vc).getModelIndex(); } @@ -1946,7 +1944,7 @@ public String getColumnName(int column) { -return dataModel.getColumnName(column); +return this.columnModel.getColumn(column).getIdentifier().toString(); } public int getEditingColumn() @@ -2077,6 +2075,47 @@ return columnModel.getColumn(columnModel.getColumnIndex(identifier)); } + /** + * Returns true if the specified cell is editable, and + * false otherwise. + * + * @param row the row index. + * @param column the column index. + * + * @return A boolean. + */ + public boolean isCellEditable(int row, int column) + { +return dataModel.isCellEditable(row, convertColumnIndexToModel(column)); + } + + /** + * Clears any existing columns from the JTable's + * [EMAIL PROTECTED] TableColumnModel} and creates new columns to match the values in + * the data ([EMAIL PROTECTED] TableModel}) used by the table. + * + * @see #setAutoCreateColumnsFromModel(boolean) + */ + public void createDefaultColumnsFromModel() + { +// remove existing columns +int columnIndex = columnModel.getColumnCount() - 1; +while (columnIndex >= 0) +{ + columnModel.removeColumn(columnModel.getColumn(columnIndex)); + columnIndex--; +} + +// add new columns to match the TableModel +int columnCount = dataModel.getColumnCount(); +for (int c = 0; c < columnCount; c++) +{ + TableColumn column = new TableColumn(c); + column.setIdentifier(dataModel.getColumnName(c)); + columnModel.addColumn(column); +} + } + public void changeSelection (int rowIndex, int columnIndex, boolean toggle, boolean extend) { if (toggle && extend) Index: javax/swing/table/DefaultTableColumnModel.java === RCS file: /cvsroot/classpath/classpath/javax/swing/table/DefaultTableColumnModel.java,v retrieving revision 1.13 diff -u -r1.13 DefaultTableColumnModel.java --- javax/swing/table/DefaultTableColumnModel.java 2 Jul 2005 20:32:51 - 1.13 +++ javax/swing/table/DefaultTableColumnModel.java 5 Jul 2005 11:11:07 - @@ -1,5 +1,5 @@ /* DefaultTableColumnModel.java -- - Copyright (C) 2002, 2004 Free Software Foundation, Inc. + Copyright (C) 2002, 2004, 2005 Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -119,9 +119,11 @@ */ public void addColumn(TableColumn col) { +if (col == null) + throw new IllegalArgumentException("Null 'col' argument."); tableColumns.add(col); invalidateWidthCache(); -fireColumnAdded(new TableColumnModelEvent(this,0,tableColumns.size())); +fireColumnAdded(new TableColumnModelEvent(this, 0, tableColumns.size() - 1)); } /** @@ -132,8 +134,10 @@ */ public void removeColumn(TableColumn col) { -int index = getColumnIndex(col); -fireColumnRemoved(new TableColumnModelEvent(this,index,0)); +int index = this.tableColumns.indexOf(col); +if (index < 0) + return; +fireColumnRemove
[cp-patches] FYI: Adding default style to the style table in the StyleContext (fixes 13617)
This should fix the recently submitted bug #13617: (https://savannah.gnu.org/bugs/?func=detailitem&item_id=13617) The reason of throwing that exception is that DefaultStyledDocument constructor tries to set the default style of the newly created StyledContext as its logical style. To get that style, it calls StyledContext.getStyle(StyleContext.DEFAULT_STYLE) and receives null in return. The attempt to add that null to the table raises an exception. Sun's implementation returns non-null value of the default style. This adds the default style to the styleTable, resulting the same behaviour. It also fixes another (unnoticed) incompatibility, related to the contents of the enumerator, returned by getStyleNames. 2005-07-05 Audrius Meskauskas <[EMAIL PROTECTED]> * javax/swing/text/StyleContext.java (defaultStyle): New field. (constructor): Add defaultStyle to the styleTable. (DEFAULT_STYLE, getStyle, getStyleNames): Documented. Index: javax/swing/text/StyleContext.java === RCS file: /cvsroot/classpath/classpath/javax/swing/text/StyleContext.java,v retrieving revision 1.4 diff -u -r1.4 StyleContext.java --- javax/swing/text/StyleContext.java 2 Jul 2005 20:32:51 - 1.4 +++ javax/swing/text/StyleContext.java 5 Jul 2005 11:28:06 - @@ -380,9 +380,17 @@ // FIXME: also not sure if these tables ought to be static (singletons), // shared across all StyleContexts. I think so, but it's not clear in // docs. revert to non-shared if you think it matters. - + + /** + * The name of the default style. + */ public static final String DEFAULT_STYLE = "default"; + /** + * The default style for this style context. + */ + NamedStyle defaultStyle = new NamedStyle(DEFAULT_STYLE, null); + static Hashtable sharedAttributeSets = new Hashtable(); static Hashtable sharedFonts = new Hashtable(); @@ -392,10 +400,15 @@ EventListenerList listenerList; Hashtable styleTable; + /** + * Creates a new instance of the style context. Add the default style + * to the style table. + */ public StyleContext() { listenerList = new EventListenerList(); styleTable = new Hashtable(); +styleTable.put(DEFAULT_STYLE, defaultStyle); } protected SmallAttributeSet createSmallAttributeSet(AttributeSet a) @@ -436,11 +449,25 @@ styleTable.remove(name); } + /** + * Get the style from the style table. If the passed name + * matches [EMAIL PROTECTED] #DEFAULT_STYLE}, returns the [EMAIL PROTECTED] #getDefaultStyle()}. + * Otherwise returns the previously defined style of + * null if the style with the given name is not defined. + * + * @param name the name of the style. + * + * @return the style with the given name or null if no such defined. + */ public Style getStyle(String name) { return (Style) styleTable.get(name); } - + + /** + * Get the names of the style. The returned enumeration always + * contains at least one member, the default style. + */ public Enumeration getStyleNames() { return styleTable.keys(); ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: Added API docs to JScrollPane
2005-07-05 Roman Kennke <[EMAIL PROTECTED]> * javax/swing/JScrollPane.java: Added API documentation for class and constructors. /Roman Index: javax/swing/JScrollPane.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JScrollPane.java,v retrieving revision 1.24 diff -u -r1.24 JScrollPane.java --- javax/swing/JScrollPane.java2 Jul 2005 20:32:48 - 1.24 +++ javax/swing/JScrollPane.java5 Jul 2005 12:04:33 - @@ -54,6 +54,9 @@ import javax.swing.plaf.UIResource; /** + * A component that embeds another component and enables it to be scrolled + * both in horizontal and vertical direction. + * * * PropertyStored in Bound? * columnHeaderscrollPane yes @@ -564,11 +567,25 @@ } + /** + * Creates a new JScrollPane without a view. The scrollbar + * policy is set to [EMAIL PROTECTED] #VERTICAL_SCROLLBAR_AS_NEEDED} and + * [EMAIL PROTECTED] #HORIZONTAL_SCROLLBAR_AS_NEEDED}. + * + * @param view the component that is embedded inside the JScrollPane + */ public JScrollPane() { this(null); } - + + /** + * Creates a new JScrollPane that embeds the specified + * view component, displaying vertical and horizontal scrollbars + * as needed. + * + * @param view the component that is embedded inside the JScrollPane + */ public JScrollPane(Component view) { this(view, @@ -576,11 +593,40 @@ HORIZONTAL_SCROLLBAR_AS_NEEDED); } + /** + * Creates a new JScrollPane without a view; The scrollbar + * policies are set to vsbPolicy and hsbPolicy. + * + * @param vsbPolicy the vertical scrollbar policy to set + * @param hsbPolicy the vertical scrollbar policy to set + * + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_ALWAYS} + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_AS_NEEDED} + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_NEVER} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_ALWAYS} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_AS_NEEDED} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_NEVER} + */ public JScrollPane(int vsbPolicy, int hsbPolicy) { this(null, vsbPolicy, hsbPolicy); } + /** + * Creates a new JScrollPane that embeds the specified + * view component; The scrollbar + * policies are set to vsbPolicy and hsbPolicy. + * + * @param vsbPolicy the vertical scrollbar policy to set + * @param hsbPolicy the vertical scrollbar policy to set + * + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_ALWAYS} + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_AS_NEEDED} + * @see [EMAIL PROTECTED] ScrollPaneConstants#HORIZONTAL_SCROLLBAR_NEVER} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_ALWAYS} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_AS_NEEDED} + * @see [EMAIL PROTECTED] ScrollPaneConstants#VERTICAL_SCROLLBAR_NEVER} + */ public JScrollPane(Component view, int vsbPolicy, int hsbPolicy) { scrollListener = createScrollListener(); ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: Jlabel revert
2005-07-05 Sven de Marothy <[EMAIL PROTECTED]> * javax/swing/JLabel.java: (JLabel): Revert previous change for JLabel(Icon) constructor. Index: javax/swing/JLabel.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JLabel.java,v retrieving revision 1.22 diff -U3 -r1.22 JLabel.java --- javax/swing/JLabel.java 2 Jul 2005 20:32:47 - 1.22 +++ javax/swing/JLabel.java 5 Jul 2005 12:13:59 - @@ -101,14 +101,14 @@ } /** - * Creates a new vertically centered, horizontally on the leading edge + * Creates a new vertically and horizontally centered * JLabel object with no text and the given icon. * * @param image The icon to use with the label. */ public JLabel(Icon image) { -this(null, image, LEADING); +this(null, image, CENTER); } /** ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Revised patch: JTable and DefaultTableColumnModel
Hi David, Attached is a revised version of the patch I proposed late last week. It adds implementations for the isCellEditable() and createDefaultColumnsFromModel() methods in JTable, and fixes some bugs uncovered while writing Mauve tests for the new methods. 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * javax/swing/JTable.java: (addColumn): retrieve correct column name, (convertColumnIndexToModel): remove check for > columnCount and let exception happen, (getColumnName): retrieve name from TableColumn, (isCellEditable): implemented, (createDefaultColumnsFromModel): implemented. * javax/swing/table/DefaultTableColumnModel.java: (addColumn): throw exception for null argument, set correct column index in TableColumnModelEvent, (removeColumn): use correct column index, (moveColumn): move the column, don't swap it. Also added argument checks, (getColumnIndex): reimplemented. I've committed more Mauve tests to cover these changes. OK to commit? Looks Ok, please commit. /Roman ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] FYI: Jlabel revert
Hi Sven, On Tue, 2005-07-05 at 14:16 +0200, Sven de Marothy wrote: > 2005-07-05 Sven de Marothy <[EMAIL PROTECTED]> > > * javax/swing/JLabel.java: > (JLabel): Revert previous change for JLabel(Icon) constructor. Could you next time please mention the actual change. Your previous change was a month ago, so it wasn't immediately apparent what was reverted. (JLabel(Icon)): Horizontal justification changed to CENTER. Thanks, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Revised patch: JTable and DefaultTableColumnModel
Roman Kennke wrote: > > Hi David, > >> Attached is a revised version of the patch I proposed late last week. >> It adds implementations for the isCellEditable() and >> createDefaultColumnsFromModel() methods in JTable, and fixes some bugs >> uncovered while writing Mauve tests for the new methods. >> >> 2005-07-05 David Gilbert <[EMAIL PROTECTED]> >> >> * javax/swing/JTable.java: >> (addColumn): retrieve correct column name, >> (convertColumnIndexToModel): remove check for > columnCount and let >> exception happen, >> (getColumnName): retrieve name from TableColumn, >> (isCellEditable): implemented, >> (createDefaultColumnsFromModel): implemented. >> * javax/swing/table/DefaultTableColumnModel.java: >> (addColumn): throw exception for null argument, set correct column >> index in TableColumnModelEvent, >> (removeColumn): use correct column index, >> (moveColumn): move the column, don't swap it. Also added argument >> checks, >> (getColumnIndex): reimplemented. >> >> I've committed more Mauve tests to cover these changes. OK to commit? >> >> > > Looks Ok, please commit. > /Roman > > > Thanks. Committed. Dave. ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Re: Absolute URL parsing bug
Hi, On Mon, 2005-07-04 at 18:07 +0100, Andrew Haley wrote: > When creating a URL: if the spec is absolute and a context URL is > supplied, we should inherit the host:port part of the context if its > protocol the same as that of the spec, but we should not inherit its > file path. Have you added the tests from your previous email to mauve? There are a lot of URL tests in mauve, any new PASSes or FAILs? > *** public final class URL implements Serial > *** 409,413 > host = context.host; > port = context.port; > - file = context.file; > userInfo = context.userInfo; > if (file == null || file.length() == 0) > --- 409,412 Isn't that last if statement now unnecessary? It seems you can set file to "/" unconditionally here. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Re: Absolute URL parsing bug
Mark Wielaard writes: > Hi, > > On Mon, 2005-07-04 at 18:07 +0100, Andrew Haley wrote: > > When creating a URL: if the spec is absolute and a context URL is > > supplied, we should inherit the host:port part of the context if its > > protocol the same as that of the spec, but we should not inherit its > > file path. > > Have you added the tests from your previous email to mauve? > There are a lot of URL tests in mauve, Ah, I didn't know that. OK, I'll try running mauve. Sigh. > any new PASSes or FAILs? > > > *** public final class URL implements Serial > > *** 409,413 > >host = context.host; > >port = context.port; > > - file = context.file; > > userInfo = context.userInfo; > >if (file == null || file.length() == 0) > > --- 409,412 > > Isn't that last if statement now unnecessary? > It seems you can set file to "/" unconditionally here. True, yes. I'm finding it pretty hard to get inside the thinking of whoever wrote this. Andrew. ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: fix opaque properties for buttons
Here comes a small fixlet that fixes how/where the opaque property for Buttons is initialized. According to some tests I have run, this property is not initialized in the buttons UI class, but instead in the buttons constructor. I have an application that uses a ToggleButton derived from JToggleButton but does not call super() in its constructors. These buttons look very ugly because they paint their background (because the opaque property is not initialized in the constructor). 2005-07-05 Roman Kennke <[EMAIL PROTECTED]> * javax/swing/JToggleButton.java (constructor): Initialize the opaque property here. * javax/swing/plaf/basic/BasicButtonUI.java (installDefaults): Don't initialize the opaque property here. /Roman Index: javax/swing/JToggleButton.java === RCS file: /CVSROOT/Jamaica/src/classpath/javax/swing/JToggleButton.java,v retrieving revision 1.11 diff -u -r1.11 JToggleButton.java --- javax/swing/JToggleButton.java 5 Jul 2005 12:28:00 - 1.11 +++ javax/swing/JToggleButton.java 5 Jul 2005 13:49:53 - @@ -79,6 +79,7 @@ super(); /* Register the accessible toggle button as a listener for item events */ addItemListener(this); + setOpaque(true); } /** Index: javax/swing/plaf/basic/BasicButtonUI.java === RCS file: /CVSROOT/Jamaica/src/classpath/javax/swing/plaf/basic/BasicButtonUI.java,v retrieving revision 1.13 diff -u -r1.13 BasicButtonUI.java --- javax/swing/plaf/basic/BasicButtonUI.java 5 Jul 2005 12:28:02 - 1.13 +++ javax/swing/plaf/basic/BasicButtonUI.java 5 Jul 2005 13:49:53 - @@ -130,7 +130,7 @@ b.setIconTextGap(defaults.getInt(prefix + ".textIconGap")); b.setInputMap(JComponent.WHEN_FOCUSED, (InputMap) defaults.get(prefix + ".focusInputMap")); -b.setOpaque(true); +//b.setOpaque(true); } protected void uninstallDefaults(AbstractButton b) ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: Added API docs to (javax.swing.) Action and AbstractAction
I committed this patch to add some API doc comments: 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * javax/swing/AbstractAction.java: updated API docs, * javax/swing/Action.java: likewise. Regards, Dave Gilbert Index: javax/swing/AbstractAction.java === RCS file: /cvsroot/classpath/classpath/javax/swing/AbstractAction.java,v retrieving revision 1.14 diff -u -r1.14 AbstractAction.java --- javax/swing/AbstractAction.java 2 Jul 2005 20:32:46 - 1.14 +++ javax/swing/AbstractAction.java 5 Jul 2005 14:51:56 - @@ -48,7 +48,8 @@ import javax.swing.event.SwingPropertyChangeSupport; /** - * AbstractAction + * A base class for implementing the [EMAIL PROTECTED] Action} interface. + * * @author Andrew Selkirk * @version1.0 */ @@ -58,12 +59,12 @@ private static final long serialVersionUID = -6803159439231523484L; /** - * enabled + * A flag that indicates whether or not the action is enabled. */ protected boolean enabled = true; /** - * changeSupport + * Provides support for property change event notification. */ protected SwingPropertyChangeSupport changeSupport = new SwingPropertyChangeSupport(this); @@ -74,7 +75,8 @@ private transient HashMap store = new HashMap(); /** - * Constructor AbstractAction + * Creates a new action with an empty string for the name. All other + * properties are initialised to null */ public AbstractAction() { @@ -82,9 +84,10 @@ } /** - * Constructor AbstractAction + * Creates a new action with the specified name. All other properties are + * initialised to null. * - * @param name TODO + * @param name the name (null permitted). */ public AbstractAction(String name) { @@ -92,10 +95,11 @@ } /** - * Constructor AbstractAction + * Creates a new action with the specified name and icon. All other + * properties are initialised to null. * - * @param name TODO - * @param icon TODO + * @param name the name (null permitted). + * @param icon the icon (null permitted). */ public AbstractAction(String name, Icon icon) { @@ -144,11 +148,12 @@ } /** - * Returns a value for a given key from the built-in store. - * - * @param key the key to get the value for - * - * @return Object + * Returns the value associated with the specified key. + * + * @param key the key (not null). + * + * @return The value associated with the specified key, or + * null if the key is not found. */ public Object getValue(String key) { @@ -156,10 +161,16 @@ } /** - * Puts a key/value pair into the built-in store. - * - * @param key the key - * @param value the value + * Sets the value associated with the specified key and sends a + * [EMAIL PROTECTED] java.beans.PropertyChangeEvent} to all registered listeners. + * The standard keys are: [EMAIL PROTECTED] #NAME}, [EMAIL PROTECTED] #SHORT_DESCRIPTION}, + * [EMAIL PROTECTED] #LONG_DESCRIPTION}, [EMAIL PROTECTED] #SMALL_ICON}, + * [EMAIL PROTECTED] #ACTION_COMMAND_KEY}, [EMAIL PROTECTED] #ACCELERATOR_KEY} and + * [EMAIL PROTECTED] #MNEMONIC_KEY}. Any existing value associated with the key will be + * overwritten. + * + * @param key the key (not null). + * @param value the value (null permitted). */ public void putValue(String key, Object value) { @@ -172,9 +183,9 @@ } /** - * isEnabled + * Returns the flag that indicates whether or not the action is enabled. * - * @return boolean + * @return The flag. */ public boolean isEnabled() { @@ -182,9 +193,11 @@ } /** - * setEnabled + * Sets the flag that indicates whether or not the action is enabled and, if + * the value of the flag changed from the previous setting, sends a + * [EMAIL PROTECTED] java.beans.PropertyChangeEvent} to all registered listeners. * - * @param enabled TODO + * @param enabled the new flag value. */ public void setEnabled(boolean enabled) { Index: javax/swing/Action.java === RCS file: /cvsroot/classpath/classpath/javax/swing/Action.java,v retrieving revision 1.9 diff -u -r1.9 Action.java --- javax/swing/Action.java 2 Jul 2005 20:32:46 - 1.9 +++ javax/swing/Action.java 5 Jul 2005 14:51:56 - @@ -1,5 +1,5 @@ /* Action.java -- - Copyright (C) 2002, 2004 Free Software Foundation, Inc. + Copyright (C) 2002, 2004, 2005 Free Software Foundation, Inc. This file is part of GNU Classpath. @@ -41,99 +41,112 @@ import java.beans.PropertyChangeListener; /** - * Action + * An action provides a convenient central point of control for some task + * that can be triggered by more than one control in a Swing user interface + * (for example, a menu item and a toolbar button). + * + * @see AbstractButton
[cp-patches] FYI: minor API doc fix in java.applet.Applet
I committed this change to fix a minor API doc problem: 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * java/applet/Applet.java: added import to fix API doc link. Regards, Dave Gilbert Index: java/applet/Applet.java === RCS file: /cvsroot/classpath/classpath/java/applet/Applet.java,v retrieving revision 1.13 diff -u -r1.13 Applet.java --- java/applet/Applet.java 2 Jul 2005 20:32:22 - 1.13 +++ java/applet/Applet.java 5 Jul 2005 14:44:03 - @@ -38,6 +38,7 @@ package java.applet; +import java.awt.Component; import java.awt.Dimension; import java.awt.GraphicsEnvironment; import java.awt.HeadlessException; ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [patch] refactor text peers to remove GtkTextComponentPeer
On Tue, 2005-07-05 at 07:43 +0200, Mark Wielaard wrote: > Hi, > > On Mon, 2005-07-04 at 19:57 -0400, Thomas Fitzsimmons wrote: > > GtkTextComponentPeer is unnecessary since the GTK widgets, GtkTextView > > and GtkEntry are not related. This patch refactors the > > GtkTextComponentPeer code into GtkTextAreaPeer and GtkTextFieldPeer. I > > applied this to mainline. > > Please be a bit more careful. This broke the build since you forgot to > change the include file generation. And --enable-Werror Should I be building with --enable-Werror? I plan to commit another patch today that will eliminate these warnings and I thought it was acceptable to introduce temporary warnings. If not, --enable-Werror should be the default. > since > classpath_gtk_component_connect_signals and the nonfocus variant are not > actually implemented. Fixed as follows: > > 2005-07-05 Mark Wielaard <[EMAIL PROTECTED]> > >* include/Makefile.am (GTKPEER_H_FILES): Removed GtkTextComponentPeer. >* include/gnu_java_awt_peer_gtk_GtkTextComponentPeer.h: Removed. >* gnu/java/awt/peer/gtk/GtkComponentPeer.java >(classpath_gtk_component_connect_nonfocus_signals): Mark argument >unused. >(classpath_gtk_component_connect_signals): Likewise. > > When we switch to gcj 4.0+ we can make --enable-regen-headers the > default with gcjh to catch this kind of thing more easily. That won't help in this case. I was building with --enable-regen-headers. For some reason, removing GtkTextComponentPeer.java does not cause gnu_java_awt_peer_gtk_GtkTextComponentPeer.h to be regenerated. I prefer how libgcj handles these headers: gtk_jni_headers = $(patsubst %.java,jniinclude/%.h,$(subst /,_,$(gtk_awt_peer_source_files))) Anyway, from now on I'll do clean builds before committing, when making header file changes. Thanks for fixing, Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] [patch] refactor text peers to remove GtkTextComponentPeer
On Tue, 2005-07-05 at 11:22 -0400, Thomas Fitzsimmons wrote: > Should I be building with --enable-Werror? I plan to commit another > patch today that will eliminate these warnings and I thought it was > acceptable to introduce temporary warnings. If not, --enable-Werror > should be the default. Yeah, I always build with --enable-Werror. I didn't make this the default since we are not warning free on all platforms. For example on cygwin there are always warnings because of issues with the system headers. It would probably be a good idea to default to --enable-Werror for CVS, but to --disable-Werror for releases. But I am afraid I would forget to change the default and cause trouble for people that don't compile much, but are on exotic platforms. Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] FYI: Jlabel revert
On Tue, 2005-07-05 at 15:37 +0200, Mark Wielaard wrote: > Could you next time please mention the actual change. Your previous > change was a month ago, so it wasn't immediately apparent what was > reverted. (JLabel(Icon)): Horizontal justification changed to CENTER. Ok. I'll do that* next time. /Sven * In sanskrit. ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] FYI: Adding read/write to javax/swing/text/JTextComponent.java (fixing bug #13640)
> "Audrius" == Meskauskas Audrius <[EMAIL PROTECTED]> writes: Audrius> +StringBuffer b = new StringBuffer(); Audrius> +int c; Audrius> + Audrius> +// Read till -1 (EOF). Audrius> +while ((c = input.read()) >= 0) Audrius> + b.append((char) c); Ouch. How about reading in bigger chunks? Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] StringReferenceCommandSet.java
> "Aaron" == Aaron Luchko <[EMAIL PROTECTED]> writes: Aaron> Here's the updated patch Aaron> ChangeLog Aaron> 2005-07-04 Aaron Luchko <[EMAIL PROTECTED]> Aaron> * gnu/classpath/jdwp/processor/StringReferenceCommandSet.java: Aaron> New file. Thanks, please commit. Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Re: Serialization: readResolve and writeReplace in parent class
> "Mark" == Mark Wielaard <[EMAIL PROTECTED]> writes: >> > I wonder if there is a strange case here where someone adds a private >> > method to the class hierarchy "later" (after all the other classes are >> > compiled). Should the private method hide the superclass methods of >> > the same signature? Perhaps this is an incompatible change, offhand I >> > forget. Mark> Agreed and there are Mauve tests to proof that. Thanks. Mark> I have no idea how this strange case looks like. Since it seems that any Mark> "later" class can only be added at the bottom of the class hierarchy. Mark> Tom, do you have an example of what you are thinking of? Suppose you have a hierarchy like: class A { blah blah readResolve() } class B extends A { } class C extends B { } You compile all of these. Then you go back and add 'private ... readResolve()' to B. Should C now really be using A's readResolve()? Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] FYI: fix opaque properties for buttons
> "Roman" == Roman Kennke <[EMAIL PROTECTED]> writes: Roman> I have an application that uses a Roman> ToggleButton derived from JToggleButton but does not call super() in Roman> its constructors. Just FYI ... you can't avoid calling super() in a constructor. If you don't call it explicitly, the compiler will add it for you. (If you call this(), some other constructor will eventually call super()). Roman> -b.setOpaque(true); Roman> +//b.setOpaque(true); Instead of commenting out code, how about deleting it and replacing it with a comment explaining why we aren't making this call. Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] Re: Serialization: readResolve and writeReplace in parent class
On Tue, 2005-07-05 at 10:41 -0600, Tom Tromey wrote: > > "Mark" == Mark Wielaard <[EMAIL PROTECTED]> writes: > Suppose you have a hierarchy like: > > class A { blah blah readResolve() } > class B extends A { } > class C extends B { } > > You compile all of these. > Then you go back and add 'private ... readResolve()' to B. How precisely would you go back? > Should C now really be using A's readResolve()? No, it should use B's readResolve(). And it will since the caching is done at runtime. Or am I missing a way to dynamically add the new method during runtime to an already loaded Class by a ClassLoader? Cheers, Mark signature.asc Description: This is a digitally signed message part ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] StringReferenceCommandSet.java
On Tue, 2005-07-05 at 10:47 -0600, Tom Tromey wrote: > > "Aaron" == Aaron Luchko <[EMAIL PROTECTED]> writes: > > Aaron> Here's the updated patch > > Aaron> ChangeLog > Aaron> 2005-07-04 Aaron Luchko <[EMAIL PROTECTED]> > Aaron> * gnu/classpath/jdwp/processor/StringReferenceCommandSet.java: > Aaron> New file. > > Thanks, please commit. Thanks, committed Aaron ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: RMIC contribution
FYI, I'm checking in Archit's rmic patch. You can see it here: http://lists.gnu.org/archive/html/classpath/2005-06/msg00045.html I wrote a ChangeLog entry for it which I hope isn't too bogus. Tom ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
Re: [cp-patches] FYI: fix opaque properties for buttons
Am Dienstag, den 05.07.2005, 10:23 -0600 schrieb Tom Tromey: > > "Roman" == Roman Kennke <[EMAIL PROTECTED]> writes: > > Roman> I have an application that uses a > Roman> ToggleButton derived from JToggleButton but does not call super() in > Roman> its constructors. > > Just FYI ... you can't avoid calling super() in a constructor. If you > don't call it explicitly, the compiler will add it for you. (If you > call this(), some other constructor will eventually call super()). Yeah, I noticed that too. The bug that I've seen was actually triggered by a different problem, the method setContentAreaFilled sets the property setOpaque() in the JDK, and this method was called in the JToggleButton derived classes constructor. I will fix this ASAP. /Roman ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] IdManager, Events
[pardon top posting] It's been over a week now, does anyone else have an opinion or does everyone agree with Bryce's comments? Keith On Mon, 2005-06-27 at 17:51 -0700, Keith Seitz wrote: > On Mon, 2005-06-27 at 17:03 -0400, Bryce McKinlay wrote: > > > Please post ChangeLog entries whenever you post a patch. I've made some > > comments below. > > ChangeLog's were posted with the original patches, and their all the > same "* filename.java: New file." I assumed such a trivial thing would > be far too silly to repost. I am incorrect, and I shall simply post > links to everything next time. > > > It seems like most JDWP events will be generated at the VM level. So, > > rather than implementing event filtering here, I think it would be > > better to pass the event filter through to the VM and have it implement > > filtering right at the source. This would be more efficient because the > > un-needed events would never be generated, instead of being created and > > then being discarded by the Java JDWP code. > > There are reasons for doing it this way, though. The biggest one is code > re-use, since any VM wanting to use JDWP support will need to implement > exactly the same thing. > > In any case, please allow me to walk through a simple example of class > preparation notification to see if we can find a better solution. > > For class prepare events, the debugger can specify the following > notification parameters: ignore count, thread, class, and class name > glob (matching and not matching). > > Regardless of how/where filtering is performed, the VM must generate/get > IDs for thread and class, compute any "hit counts", and get the string > representation of the class name (if it does not have that data > available). > > That's a whole lot of work already mandated by filtering. IMO pushing > filtering down into the VM is not going to provide sufficient incentive > to outweigh re-usability. > > The only alternative solution I could devise involved looking up, e.g., > the Thread when an event request comes in, storing a Soft/WeakReference > to that Object and then comparing that against the Thread supplied with > the event notification. I guess there is sufficient incentive to move to > this, even though I had misgivings about this earlier (but it's been so > long, I don't recall why). > > Nonetheless, none of this addresses your concerns. > > Perhaps a bigger piece of the puzzle might help elide a new solution? > For ClassPrepareEvent, I have a notification in gcj which looks > something like (pardon lots of pseudo-code): > > if (gnu::classpath::jdwp::Jdwp::isDebugging) > { >using namespace gnu::classpath::jdwp; >java::lang::Thread* t = currentThread(); >java::lang::Class* clazz = defining class; >int flags = class preparation flags; >event::ClassPrepareEvent* event = new event::ClassPrepareEvent (t, clazz, > flags); >Jdwp.notify (event); > } > > Jdwp.notify actually attempts to ascertain what request from the > debugger actually wanted the notification. If no registered notification > exists, the event is never converted into bytes and sent. Otherwise, the > request ID is returned and the event is converted to bytes and sent over > the transport. > > I'm afraid I just don't see where any appreciable performance gain is > going to be made, other than running native+JNI vs. (possibly) > interpreted java. > > > Note that WeakHashMap matches keys based on equals(), not object > > identity. This might not be what you want for a debugger, since two > > "equal" but different objects may be an important distinction for the > > user. Of course, IdentityHashMap isn't what we want either because the > > references won't be weak. I think that some custom data structure is > > needed here. > > > > Then again, perhaps this isn't so important for all cases, so I wouldn't > > be against this patch if you want to commit this as-is in the interim, > > then come back and fix it later. > > I don't really know. I don't recall reading anything in the JDWP > specification alluding to a distinction. If worse comes to worse, we can > revive and revise the ReferenceKey patch and make it work. > > Keith ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] [patch] prevent bogus casts in DefaultKeyboardFocusManager.dispatchEvent
Hi, This patch adds the check Jeroen and I discussed in this thread: http://lists.gnu.org/archive/html/classpath-patches/2005-06/msg00363.html It should prevent bogus casts to Window during KeyEvent dispatch. I committed this to mainline. Tom 2005-07-05 Thomas Fitzsimmons <[EMAIL PROTECTED]> * java/awt/DefaultKeyboardFocusManager.java (dispatchEvent): Check parent and target before casting target to Window. Index: java/awt/DefaultKeyboardFocusManager.java === RCS file: /cvsroot/classpath/classpath/java/awt/DefaultKeyboardFocusManager.java,v retrieving revision 1.14 diff -u -r1.14 DefaultKeyboardFocusManager.java --- java/awt/DefaultKeyboardFocusManager.java 2 Jul 2005 20:32:24 - 1.14 +++ java/awt/DefaultKeyboardFocusManager.java 5 Jul 2005 19:10:53 - @@ -187,20 +187,26 @@ // Keep track of this window's focus owner. -// Find the target Component's top-level ancestor. +// Find the target Component's top-level ancestor. target +// may be a window. Container parent = target.getParent (); while (parent != null && !(parent instanceof Window)) parent = parent.getParent (); -Window toplevel = parent == null ? - (Window) target : (Window) parent; +// If the parent is null and target is not a window, then target is an +// unanchored component and so we don't want to set the focus owner. +if (! (parent == null && ! (target instanceof Window))) + { +Window toplevel = parent == null ? + (Window) target : (Window) parent; -Component focusOwner = getFocusOwner (); -if (focusOwner != null -&& ! (focusOwner instanceof Window)) - toplevel.setFocusOwner (focusOwner); +Component focusOwner = getFocusOwner (); +if (focusOwner != null +&& ! (focusOwner instanceof Window)) + toplevel.setFocusOwner (focusOwner); + } } else if (e.id == FocusEvent.FOCUS_LOST) { ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] IdManager, Events
Hi Keith, Sorry I havn't got back to you sooner. Re filtering: I think that whether filtering at the Java level makes a significant difference to performance depends on the proportion of events that are filtered in a typical debugging session. If, say, 90%+ of events are usually filtered out and then discarded, then there might be a significant cost in all the object allocations for those events. But if filtering isn't all that common, then it shouldn't be a big deal. Its also worth considering that doing the filtering work will carry a significant cost in itself: once we are doing things like matching class names against globs, then the cost of generating and then discarding the event isn't going to be all that much greater in comparison. Re: WeakHashMap: basically the issue here is that if we continue to use WeakHashMap, it will be possible to have two distinct objects map to the same ObjectID. Although perhaps not common, I'm sure that this behaviour will cause problems/confusion/incorrect results in some cases and will certainly not be consistent with other JWDP implementations. Here's a solution to consider: While it will still be necessary to maintain some hashtable structure for Object->ObjectID mappings, it would be relatively trivial to add unique Class IDs and Thread IDs into the VMs class and thread structs, thus eliminating the overhead of maintain extra tables and looking up the IDs all the time. I suspect that this would also be easy to add in other VMs if they don't have it already. So, instead of managing the ID<->Object/Class/Thread mappings in Java, we could define an interface something along the lines of: VMDebug.getClassID(java.lang.Class) VMDebug.getThreadID(java.lang.Thread) VMDebug.getObjectID(java.lang.Object) VMDebug.getClass(ClassID) VMDebug.getThread(ThreadID) VMDebug.getObject(ObjectID) This interface could be expanded to provide a lower-level debugging interface that bypasses reflection (due to use of reflection operations causing class initialization). In turn, this interface could be implemented in terms of the JVMTI. We'd need to implement at least a significant subset of the JVMTI in libgcj, but that should give you everything you need to implement JDWP in one clean, documented interface. Note that JVMTI contains the concept of "object tags" which seems roughly analogous to JDWP IDs. In any case, my comments were not intended as a rejection of your patch, but rather just to highlight a few issues that I think need to be considered/addressed eventually. Its better to get the code in now even if its a work in progress that might be changed later. So please go ahead and check it in! Thanks Bryce Keith Seitz wrote: [pardon top posting] It's been over a week now, does anyone else have an opinion or does everyone agree with Bryce's comments? Keith On Mon, 2005-06-27 at 17:51 -0700, Keith Seitz wrote: On Mon, 2005-06-27 at 17:03 -0400, Bryce McKinlay wrote: Please post ChangeLog entries whenever you post a patch. I've made some comments below. ChangeLog's were posted with the original patches, and their all the same "* filename.java: New file." I assumed such a trivial thing would be far too silly to repost. I am incorrect, and I shall simply post links to everything next time. It seems like most JDWP events will be generated at the VM level. So, rather than implementing event filtering here, I think it would be better to pass the event filter through to the VM and have it implement filtering right at the source. This would be more efficient because the un-needed events would never be generated, instead of being created and then being discarded by the Java JDWP code. There are reasons for doing it this way, though. The biggest one is code re-use, since any VM wanting to use JDWP support will need to implement exactly the same thing. In any case, please allow me to walk through a simple example of class preparation notification to see if we can find a better solution. For class prepare events, the debugger can specify the following notification parameters: ignore count, thread, class, and class name glob (matching and not matching). Regardless of how/where filtering is performed, the VM must generate/get IDs for thread and class, compute any "hit counts", and get the string representation of the class name (if it does not have that data available). That's a whole lot of work already mandated by filtering. IMO pushing filtering down into the VM is not going to provide sufficient incentive to outweigh re-usability. The only alternative solution I could devise involved looking up, e.g., the Thread when an event request comes in, storing a Soft/WeakReference to that Object and then comparing that against the Thread supplied with the event notification. I guess there is sufficient incentive to move to this, even though I had misgivings about this earlier (but it's been so long, I don't rec
[cp-patches] [RFA/JDWP] ReferenceTypeCommandSet.java
Okay, here's the first CommandSet with some real heft, http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_ReferenceType There are a couple interesting points for this patch. The first is that upon Keith's suggestion any Command which has something to do with generics (in this case just queries for the generic signature) we just throw a NotImplementedException since we don't do any generics. This shouldn't cause any problems because there's always a non-generic counterpart though we could theoretically also return an empty string for the generic signature. There's also a couple places I use classes from java.lang.reflect This is a bit of a problem as it's been pointed out if in the instance of a command like http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_ReferenceType_GetValues Using the Field object will cause the Class to initialize which would be changing the execution of the program. Tom Tromey suggested we could replace the java.lang.relfect classes we use by our own classes later on that reach into the JVM and avoid this problem. Aaron ChangeLog 2005-07-05 Aaron Luchko <[EMAIL PROTECTED]> * gnu/classpath/jdwp/processor/ReferenceTypeCommandSet.java:New file. --- /dev/null 2005-06-09 16:29:11.371620296 -0400 +++ gnu/classpath/jdwp/processor/ReferenceTypeCommandSet.java 2005-07-05 15:30:48.0 -0400 @@ -0,0 +1,324 @@ +/* ReferenceTypeCommandSet.java -- lass to implement the ReferenceType + Command Set + Copyright (C) 2005 Free Software Foundation + +This file is part of GNU Classpath. + +GNU Classpath is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2, or (at your option) +any later version. + +GNU Classpath is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GNU Classpath; see the file COPYING. If not, write to the +Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA +02111-1307 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. */ + + +package gnu.classpath.jdwp.processor; + +import gnu.classpath.jdwp.IVirtualMachine; +import gnu.classpath.jdwp.Jdwp; +import gnu.classpath.jdwp.JdwpConstants; +import gnu.classpath.jdwp.exception.InvalidFieldException; +import gnu.classpath.jdwp.exception.JdwpException; +import gnu.classpath.jdwp.exception.JdwpInternalErrorException; +import gnu.classpath.jdwp.exception.NotImplementedException; +import gnu.classpath.jdwp.id.IdManager; +import gnu.classpath.jdwp.id.ObjectId; +import gnu.classpath.jdwp.id.ReferenceTypeId; +import gnu.classpath.jdwp.util.JdwpString; +import gnu.classpath.jdwp.util.Signature; +import gnu.classpath.jdwp.util.Value; + +import java.io.DataOutputStream; +import java.io.IOException; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.ByteBuffer; + +/** + * A class representing the ReferenceType Command Set. + * + * @author Aaron Luchko <[EMAIL PROTECTED]> + */ +public class ReferenceTypeCommandSet implements CommandSet +{ + + // Our hook into the jvm + private final IVirtualMachine vm = Jdwp.getIVirtualMachine(); + + // Manages all the different ids that are assigned by jdwp + private final IdManager idMan = Jdwp.getIdManager(); + + public boolean runCommand(ByteBuffer bb, DataOutputStream os, byte command) + throws JdwpException + { +try + { +switch (command) + { + case JdwpConstants.CommandSet.ReferenceType.SIGNATURE: +executeSignature(bb, os); +break; + case JdwpConstants.CommandSet.ReferenceType.CLASS_LOADER: +executeClassLoader(bb, os); +break; + case JdwpConstants.CommandSet.Reference
[cp-patches] Re: [RFA/JDWP] ReferenceTypeCommandSet.java
On Tue, 2005-07-05 at 15:32 -0400, Aaron Luchko wrote: > There are a couple interesting points for this patch. The first is that > upon Keith's suggestion any Command which has something to do with > generics (in this case just queries for the generic signature) we just > throw a NotImplementedException since we don't do any generics. This > shouldn't cause any problems because there's always a non-generic > counterpart though we could theoretically also return an empty string > for the generic signature. Generics is a 1.5 feature. Anyone have a problem with implementing JDWP 1.4 first? 1/2 :-) > There's also a couple places I use classes from java.lang.reflect > This is a bit of a problem as it's been pointed out if in the instance > of a command like > http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_ReferenceType_GetValues > Using the Field object will cause the Class to initialize which would be > changing the execution of the program. Tom Tromey suggested we could > replace the java.lang.relfect classes we use by our own classes later on > that reach into the JVM and avoid this problem. The question is, though, is this really a problem? In order to use ReferenceType commands, the debugger must have already requested a reference ID for the given class. AFAICT the only way to get a reference type ID for a class is to register for some event. Eclipse, for example, uses class prepare events. Anyway, the existence of a reference type ID would mean that the class must have been loaded/initialized at some time, no? At the very least, as Tom suggests, we could certainly use this a first attempt, replacing when/if necessary. Keith ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] FYI: fix for JTable.getColumnName()
A patch I committed earlier today had a fix for the getColumnName() method in JTable that wasn't quite right. This patch fixes it properly: 2005-07-05 David Gilbert <[EMAIL PROTECTED]> * javax/swing/JTable.java (getColumnName): return name from column in data model. Regards, Dave Gilbert Index: javax/swing/JTable.java === RCS file: /cvsroot/classpath/classpath/javax/swing/JTable.java,v retrieving revision 1.31 diff -u -r1.31 JTable.java --- javax/swing/JTable.java 5 Jul 2005 13:49:25 - 1.31 +++ javax/swing/JTable.java 5 Jul 2005 20:13:09 - @@ -1944,7 +1944,8 @@ public String getColumnName(int column) { -return this.columnModel.getColumn(column).getIdentifier().toString(); +int modelColumn = columnModel.getColumn(column).getModelIndex(); +return dataModel.getColumnName(modelColumn); } public int getEditingColumn() ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] ReferenceTypeCommandSet.java
Keith Seitz wrote: There's also a couple places I use classes from java.lang.reflect This is a bit of a problem as it's been pointed out if in the instance of a command like http://java.sun.com/j2se/1.5.0/docs/guide/jpda/jdwp/jdwp-protocol.html#JDWP_ReferenceType_GetValues Using the Field object will cause the Class to initialize which would be changing the execution of the program. Tom Tromey suggested we could replace the java.lang.relfect classes we use by our own classes later on that reach into the JVM and avoid this problem. The question is, though, is this really a problem? In order to use ReferenceType commands, the debugger must have already requested a reference ID for the given class. AFAICT the only way to get a reference type ID for a class is to register for some event. Eclipse, for example, uses class prepare events. Anyway, the existence of a reference type ID would mean that the class must have been loaded/initialized at some time, no? Class preparation is distinct from initialization. Preparation is effectively linking the class: laying out its fields, initializing class data structures in the VM, etc. Initialization involves actually running Java code to initialize the classes static fields, and any "static" code blocks the class may have. The point at which initialization occurs is strictly defined by the JLS, and its pretty easy to write a program that will behave differently if initializations are not performed in the "right" order. That means that application could be behave differently when it is debugged. So, I think ultimately we will need to address this issue. Whether it matters in practice for the majority of debug sessions, I don't know. At the very least, as Tom suggests, we could certainly use this a first attempt, replacing when/if necessary. That sounds good to me. Bryce ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] ReferenceTypeCommandSet.java
On Tue, 2005-07-05 at 16:15 -0400, Bryce McKinlay wrote: > Class preparation is distinct from initialization. Preparation is > effectively linking the class: laying out its fields, initializing class > data structures in the VM, etc. Initialization involves actually running > Java code to initialize the classes static fields, and any "static" code > blocks the class may have. The point at which initialization occurs is > strictly defined by the JLS, and its pretty easy to write a program that > will behave differently if initializations are not performed in the > "right" order. So can a class be prepared by a user application and not be initialized? Keith ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: [RFA/JDWP] ReferenceTypeCommandSet.java
Keith Seitz wrote: On Tue, 2005-07-05 at 16:15 -0400, Bryce McKinlay wrote: Class preparation is distinct from initialization. Preparation is effectively linking the class: laying out its fields, initializing class data structures in the VM, etc. Initialization involves actually running Java code to initialize the classes static fields, and any "static" code blocks the class may have. The point at which initialization occurs is strictly defined by the JLS, and its pretty easy to write a program that will behave differently if initializations are not performed in the "right" order. So can a class be prepared by a user application and not be initialized? Yes, or at least it is possible for a user application to get a reference to a Class which is not initialized (eg the 3-argument Class.forName() or using ".class" syntax in Java source code). The point at which preparation actually occurs is determined by the implementation, with the restriction (obviously) that it must be prepared before it can be initialized. The failure case I can imagine is: - VM prepares a class (but the application hasn't yet initialized it) - JDWP sends ClassPrepare event to debugger - Debugger queries the value of a static field in the class - JDWP uses java.lang.reflect to respond to query ---> Class initialized too early Thinking about it more, I think it should be possible to avoid this issue by keeping track of whether a class is initialized, and not attempting to get static field values for uninitialized classes (there might be a few other cases where initialization could be triggered by reflection ops, too). The VM could implement something like VMDebug.isInitialized(jclass) for this. Bryce ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches
[cp-patches] Re: Absolute URL parsing bug
Andrew Haley wrote: The case that we get wrong is this one: $ java TestURLs 'jar:file:/ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl' 'jar:file:ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl' jar:file:ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl None of these are valid URLs as documented in the javadoc. It matches the URI specification if you view everything after the scheme (i.e. "file:/ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl") as an 'opaque_part'. There is no concept of a "nested URL" and "jar:file:" is not a valid scheme. $ gij TestURLs 'jar:file:/ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl' 'jar:file:ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl' jar:file:/ejbjars/ws.jar!/META-INF/wsdl/file:ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl So, in that case the context should be ignored. In fact, it seems like the context is ignored for all "jar:file" and "file:" URLs: We should ignore the context for *all* absolute URLs - i.e. any URL that has a scheme - accordingto the URI spec. But the Javadoc doc URL(URL, String) says "If the scheme component is defined in the given spec and does not match the scheme of the context, then the new URL is created as an absolute URL based on the spec alone."I assume the "and does not match" clause is documenting a hack (bug) in the implementation - which doesn't match the URI spec. (Hacks like this may be one reason why they decided to start from scratch with a new URI class.) I.e. java TestURLs http:/a/b/c http:d/e prints: http:/a/b/d/e but according to the RFC spec it should be: http:d/e The URI.resolve method gets this right. A suggested (untested) fix might be something like: int colon = spec.indexOf(':'); int slash = spec.indexOf('/'); if (colon > 0 && (colon < slash || slash < 0) && (protocol == null || protocol.length() <= colon || ! protocol.equalsIgnoreCase(spec.substring(0, colon context = null; But in the case of a URL that is not qualified with any protocol at all we need the context: Er, no. See the 'TestURLs http:/a/b/c http:d/e' example. $ java TestURLs 'jar:file:/ejbjars/ws.jar!/META-INF/wsdl/ssbEndpoint.wsdl' foo jar:file:/ejbjars/ws.jar!/META-INF/wsdl/foo Classpath gets that right too. So, the only thing we seem to get wrong is the parsing of a 'jar:file:' URL when a 'jar:' context is supplied. Do you have any reason to believe that "jar:" or "file:" URLs get any special treatment? I see none. -- --Per Bothner [EMAIL PROTECTED] http://per.bothner.com/ ___ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches