On Aug 5, 2010, at 12:14 PM, Krzysztof Kosiński wrote:
W dniu 5 sierpnia 2010 09:28 użytkownik Jon Cruz <jon@...18...> napisał:
One note, though. I see that the patch is using a "MAX" macro. Now that Inkscape is using C++, we should try to use std::max() instead. Hmm... and the placement of the "+ 1" outside of the max seems a little odd.
I have a suspicion that either MoveWindow breaks when the width is the same as before, or GetWindowRect returns bogus values. The "+1" is to avoid the first case.
Ahh... then it really needs to be broken out. That is, collapsing things to a single sub-expression inside of a function call hides what the intent is, and can contribute to bugs.
There are also problems with using MAX or std::max at all. This is really not the case where that is desired, as the value we're checking against should not be the large one we use for a default.
But probably the most important thing is to try to explicitly test your suspicions. At the lest, separating the work-arounds so that an end user could easily comment out just one of them would really help with testing.
Oh, and now that I think of it, if it is helpful to cap the width, it might also be handy to watch the height. That's one area where GTK programs get dinged for not respecting the current dynamic 'usable' area, etc.
Height was not a problem in the bug report, but it might make some sense to add it. I think that native dialogs should be removed in 0.49 because they are a maintenance nightmare, and make implementing features like the "embed" checkbox in the import dialog more difficult than it should be.
Yes, height was also an issue. Someone explicitly called out the zero-height problem in comment #32 there.