On 03/29/2013 06:16 AM, Henri Verbeet wrote:
I think the idea is basically ok, but I do have some comments:
I think test_window_style() is a more appropriate place for the tests,
and you should add them to the variants in ddraw as well. Ddraw
applications in particular are very fragile in this regard. Note that
the test_window_style() tests are fairly similar to what your tests
do, except that they don't test restoring the window as such. That
test also shows that we're actually not supposed to touch the styles
at all, but that's hard to make work without calling into winex11.
Ack, you're right! I saw the test_window_style() but second-guessed
myself and put it into test_reset() instead... That was foolish of me.
Thanks.
- SetWindowPos(window, HWND_TOP, 0, 0, w, h, SWP_FRAMECHANGED |
SWP_SHOWWINDOW | SWP_NOACTIVATE);
+ SetWindowPos(window, HWND_TOPMOST, 0, 0, w, h, SWP_FRAMECHANGED |
SWP_SHOWWINDOW | SWP_NOACTIVATE);
This is really a separate change.
I'm a bit confused, then: Without this change, the exstyle test breaks,
and the guidelines request that every patch be atomic. They also request
that all tests be included in the same patch.What is the proper
procedure for this when it comes to multipatch patchsets?
Off the top of my head, I would guess it's one of these:
1. The "all tests in the same patch" rule is only appropriate for
single-patch submissions, so in multipatch patchsets, it's acceptable to
include a final patch with nothing but tests.
2. Only introduce the tests in the last patch of the patchset. (That is,
add them all at once, but only when all of the necessary changes have
been made to allow the tests to work first.)
3. Include each test in the first patch that will allow the test to
pass. (That is, add them incrementally.)
4. Add the tests as todo_wine in the first patch, then remove the
todo_wine in later patches as each test becomes functional. (That is,
add them all at once as todo_wine, and mark them as non-todo incrementally.)
+ device->style ^= (device->style^style) & WS_VISIBLE;
Spaces around the (second) ^, please.
Done. :)
Thank you,
Sam
