<AWT Dev> [PATCH] Cleanup AWT peer interfaces

[Artem Ananiev]

Andrei V. Dmitriev wrote:
> Hi Roman,
> 
> Cool! I'm comfortable with this fix now.
> 
> We still need second approve vote to push this change in.
> Artem...? Others? ;)

Well, I'm fine with the proposed changes in general. Still, some JavaDoc 
comments looks to be too implementation-specific. For example, most of 
Container's methods contain a statement like this:

This is called from {@link ...}, before/after ...

and they really called from the specified places, however this may not 
be true in future. I'd remove all these statements from JavaDoc, leaving 
only behavioral description.

Thanks,

Artem

> Thanks,
>   Andrei
> 
> Roman Kennke wrote:
>> Hi Andrei,
>>
>> Finally I came around to fix the suggested stuff. See comments inline.
>>
>>> 1) src/share/classes/java/awt/peer/CheckboxMenuItemPeer.java
>>>
>>> +import java.awt.CheckboxMenuItem;
>>> if that's really needed? I thought that {@link ...} doesn't require 
>>> such stuff.
>>
>> I don't know. If you fully qualify the stuff in the {@link } it's not
>> needed. Should I fully-qualify things everywhere or let the import in
>> place? I don't care really.
>>
>>
>>> 2) src/share/classes/java/awt/peer/ContainerPeer.java
>>> There is a typo in the second word:
>>> -     * Indicates availabiltity of restacking operation in this 
>>> container.
>>> +     * Indicates availability of restacking operation in this 
>>> container.
>>
>> Fixed.
>>
>>> 3) Common thing.
>>> <code>true</code> have a shorter synonym since JDK 5: {@code true}
>>
>> Fixed.
>>
>>> 4) src/share/classes/java/awt/peer/RobotPeer.java
>>> I noticed that you prefer not to leave "public" modifier in an 
>>> interface. But here you are leaving all of them.
>>
>> Fixed.
>>
>>> 5) src/share/classes/java/awt/peer/RobotPeer.java
>>> public int getNumberOfButtons(); has left w/o any comments.
>>
>> Fixed. I'm not sure if I got the sematics right.
>>
>>> 6) src/share/classes/java/awt/peer/ComponentPeer.java
>>> +    /**
>>> +     * Called by {@link EventQueue#coalescePaintEvent} to let the 
>>> component
>>> +     * peer coalesce paint events.
>>> +     *
>>> +     * @param e the paint event to consider to coalesce
>>> +     *
>>> +     * @see EventQueue#coalescePaintEvent
>>> +     */
>>> +    void coalescePaintEvent(PaintEvent e);
>>>
>>> I'd say it's not "to let .... coalesce paint events", but "to 
>>> coalesce paint events".
>>
>> Fixed.
>>
>>> 7) at the same class.
>>> You wrote:
>>> +    // TODO: Maybe change this to force Graphics2D, since many 
>>> things will
>>> +    // break with plain Graphics nowadays.
>>> +    Graphics getGraphics();
>>>
>>> Do you know a scenario to show what's exactly might be broken. We 
>>> probably need to introduce another peer for that, right?
>>
>> I already explained my reason in earlier mails. You think this is
>> reasonable?
>>
>>
>>> 8) Similar to 7):
>>> +    // TODO: Maybe make that return a BufferedImage, because some 
>>> stuff will
>>> +    // break if a different kind of image is returned.
>>> +    Image createImage(int width, int height);
>>
>> Dito.
>>
>> This time I did the work on top of the -awt workspace and created a
>> webrev for your reviewing pleasure:
>>
>> http://kennke.org/~roman/docpeers/webrev/
>>
>> Thanks for having a look at it again,
>>
>> Roman
>>