<AWT Dev> Review request: 6689983 (reevaluate our inset-related code in XAWT)

[Anthony Petrov]

Hi Oleg,

On 07/22/2009 11:18 AM, Oleg Sukhodolsky wrote:
> Ok, so there are two main purposes:
> - to move most of insets-guessing code to XDecorated Peer.
> - better support for modern WMs

Well... The CR might be honestly renamed to "Re-factor the 
insets-related code... completely". :) While I agree that this is far 
from perfect to rewrite such a big chunk of code, I should also admit 
that the existing code is hardly maintainable.


> Both look nice and I agree that it is worth to implement both.  But I
> would suggest to do them separately.
> This way it would be easier to review/understand changes.  Also I have
> noticed several other changes
> which are not related to main purposes (e.g. support for restarting
> WM, support for changing insets on-fly),
> and I think it would be better to do these changes as separate commits
> too. Small changes are easier to test and review, and,
> I believe, you will find some tests (not necessary automatic) for
> these changes ;)
> 
> I understand, that such approach increase amount of work you need to
> do, but it simplifies reviewing and
> so improve quality of the code.  This why I suggest it.

I agree with the point: the smaller the changes, the easier to review. 
But due to the nature of the current state of the fix I doubt if it is 
doable now. E.g., I could drop the XWM.reset() method and its invocation 
from the XDecPeer, but is it worthwhile to file a separate CR for just 
about 5-10 lines of quite clear code that is quite related to the aims 
of the fix? Note that all these small pieces would reduce the fix size 
insignificantly (maybe ~3-5% at most?), thus not actually reducing the 
complexity of the fix.


PS. To keep you up to date: the fix has been tweaked quite a lot since 
the version #0. Apart from applying the suggestions expressed on the 
mailing list, some problems were fixed when running on a remote DISPLAY 
on the Cygwin X server with their "native" window manager. Though we do 
not officially support this configuration, we still want to work 
acceptably in some most popular cases (notably, when running the 
NetBeans IDE). Currently I'm in the process of re-running the regression 
tests. Hopefully the next version will follow sometime next week. Please 
stay tuned. And I want to thank you again for your passionate, 
unquenchable interest in this area! This is very appreciated!

--
best regards,
Anthony