Lars Gullik Bjønnes wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| > | + lyx::frontend::WorkArea * work_area_;
| > Do we want bald pointers? Even just as cache?
|
| I can't use reference because I want to be able to change work_area_
| at run-time. This work_area_ is not created nor deleted here so it is
| a _safe_ use of pointer. Feel free to suggest something else that
| retain polymorphisms.
boost::shared_ptr<lyx::frontend::WorkArea, no_delete> work_area_;
IMO, this is just noise. This work_area_ is already enclosed in a
shared_ptr (look at TheGui.[Ch]). What's the use of this shared_ptr
here? What's wrong in passing a pointer if we know exactly where it is
created and deleted (and nowhere else)? Are you affraid of some hacker
doing a "delete work_area_" somewhere?
It you answer "yes" to the last question, I really don't share you fear.
If you want we can ban the use of all "delete" use through out the
code. Then a simple "grep -r delete" will tell you if there some
forbidden code somewhere.
(you have to create the deleter though, no_delete is just an example)
[...]
| > This "TheGui" thing sounds/looks strange. what is it? Can it get a
| > more descriptive name?
|
| Gui.h is the virtual interface to everything Gui related:
| - clipboard
| - work areas
| - views (including menubar, etc)
| - etc
|
| There should be only _one_ instance of Gui object, hence the name:
| _The_ Gui
I think a more descriptive name should be found. And don't have any
real good idea, but TheGui is vague... but... if only once instance,
why not as a singleton? GView::getGuiInstance().<do stuff with it>.
Please No. The views will be part of TheGui in my next cleanup round
(see already sent patch). Ultimately I want only one global variable:
Application * theApp;
TheGui will be accessible through theApp->gui();
but it is the class name that is vague... hmm... even MainGui is
better imho. (MainGuiClass)
IMHO MainGui is wrong because there shall be just one unique Gui,
MainGui suggest that there could be more than one. Please Note that I
have change the naming 4 times already and I've asked repeatedly your
opinion about that.
| > | -void GWorkArea::putClipboard(string const & str) const
| > | +void GWorkArea::putClipboard(string const & str)
| > | {
| > Why the const removals?
|
| I can put it back but the const was and still is _wrong_. PutClipbard
| modify the clipboard and for now, the gtk clipboard is part of
| GWorkArea so it should not be const. It used to work without the const
| because the clipboard is not really part of it and putClipboard() just
| modify some global variable.
but const was also removed from getClipboard, wasn't it?
Nope it is still there.
| Are you suggesting tree additional file that will be compiled
| depending on the platform? Something like:
| - lstring_win.C
| - lstring_mac.C
| - lstring_x11.C
|
| I am fine with that but I don't know enough about m4 to do this. This
| can be done after the merging.
You don't have to know any m4 to do this.
but you might ahve to set a automake conditional
Let's postpone this until after the merge please.
| So I correct those thing that you don't like can I merge to trunk?
Eager now? Yes.
Alleluia!
:-)
Thanks for the comments,
Abdel.
