Re: Patch: add ActionEvent and TextEvent handling to TextComponent peers

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Tom" == Thomas Fitzsimmons <fitzsim@redhat.com> writes:

Tom> Please review/comment.

Looks good.  Just a couple minor points/questions.

Tom> -  public void setCaretPosition (int pos)
Tom> -  {
Tom> -    set ("text_position", pos);
Tom> -  }

I haven't looked at the peers in quite a while.  As I recall there are
old and new styles of setting attributes on a widget.  Do you know
which is which?  Or why we should prefer one over the other?

My recollection is that the string-based one is the "new" API.  I
don't care so much about that -- I just would like to know where we're
headed, if possible.

Tom> +  public native void setCaretPosition (int pos);

I don't see new native implementations of setEditable or
setCaretPosition.

Tom> Index: java/awt/event/ActionEvent.java
Tom> -    s.append(actionCommand).append(",when=").append(when).append("modifiers");
Tom> +    s.append(actionCommand).append(",when=").append(when).append(",modifiers");

This bit is definitely ok; I just wanted to point out that something
like this can go in as an obvious change.  Just post a note and check
it in separately; no approval required.

I took a quick look at this method, though, and I think the
setLength() call should be removed.  It seems to me that the code is
intended to overwrite the first `+' with and `=', but instead we end
up with "modifiers=+foo", due to the setLength.  Am I reading this
right?

Tom> +  if ((vk >= 0x20 && vk <= 0x7e)	/* Most printable keysyms on a standard US keyboard. */

Just FYI, the usual GNU rules apply.  We should wrap lines at 80
characters.

Tom> -  jclass gtkcomponentpeer, gtkwindowpeer, gtkscrollbarpeer, gtklistpeer, gtkmenuitempeer;
Tom> +  jclass gtkcomponentpeer, gtkwindowpeer, gtkscrollbarpeer, gtklistpeer, gtkmenuitempeer, gtktextcomponentpeer;

Likewise here, though I realize this one isn't your fault.

Pure formatting cleanups can also go in as separate patches without
approval under the obvious rule.


This will have to go in to Classpath when it goes into libgcj.

Tom