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
