Lars Gullik Bjønnes wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| * frontends/Timeout:
| - emit() renamed to emitSignal() to avoid compiler confusion with Qt
| emit() function (change propagated to all frontend).
This I do not like.
I agree on the principle but, come on, we don't have to be religious
about such stupid things. The timeout change is much easier and quicker
that the change you are proposing.
Qt deserves a lot of flak for pushing such common names into
the global namespace.
The qt side of things should fix this, not other code.
#ifdef emit
#define qt_emit emit
#undef emit
#endif
And then use qt_emit in all qt code.
No, there is a boost macro compatibility flag that I plan to use in the
future for that. This flag will need a change of the "emit", "signals"
and "slot" macros to "Q_EMIT", "Q_SIGNALS" and "Q_SLOT" or something
like that. In the mean time, please accept my timeout::emit change that
is really not a big deal.
As for the patch... you are changing too much at the same time, at
least I feel overwhelmed, and unable to review the patch properly.
I am sorry but this is what happens when you do API changes. Patch
reviewing is not a sane method in this case because there are a lot of
code moving. You have to apply the patch and _read_ the code...
Beside, the svn log is not clear enough?
It must be possible to split this patch into more manageable pieces.
Maybe but I really don't see the advantage and it will require me to
invest much more time working on this for nothing. Please try to read
the code instead. If you prefer a branch, we could do that also.
| + /// needs_redraw_ accessor
| + /**
| + This accessor is there for the WorkArea class use. The WorkArea
| + must set needsRedraw to false once screen redrawing is done!
| + \retval true is a redraw is needed
| + \retval fasle otherwise
| + */
| + bool & needsRedraw();
A binary value shoudl have a getter and a setter IMHO, not direct
access to the underlying variable.
I agree and I knew that I would get such comment ;-)
I'll change that.
Abdel.
