Le mar 06/04/2004 à 15:17, PUYDT Julien a écrit : > On mar, 2004-04-06 at 14:56, Damien Sandras wrote: > > 1) The chat window is a GtkWidget which is part of the GmWindow. It > > means that you should have a pointer to that GtkWidget inside GmWindow > > and that you should not have functions like GnomeMeeting::Process > > ()->GetTextChatWindow (). Such functions are only for top-level windows, > > not for internal widgets inside those windows. > > Well, there was a GetTextChat function, you know... I just renamed it to > describe what it does. > The GetTextChat function returned a structure and was thus needed. Now that we are moving to a GtkWidget object type, it has to be removed as we have back a normal GtkWidget part of the GmWindow. I never liked the fact that there was a public structure for something as simple as the text chat, but I am not the author of the text chat code... > > 2) + /* delete (chat_window); unneeded: embedded in the main window! */ > > If it is unneeded, why don't you remove the line instead of committing > > it to CVS? ;) > > Two reasons: > 1) that's documentation; So you should add this too : /* delete (docklet); unneeded embedded in the main window! */ /* delete (video_settings_frame); unneeded embedded in the main window! */ /* delete (audio_settings_frame); unneeded embedded in the main window! */ /* delete (statusbar); unneeded embedded in the main window! */ /* delete (remote_name); unneeded embedded in the main window! */ /* delete (splash_win); unneeded embedded in the main window! */ /* delete (combo); unneeded embedded in the main window! */ /* delete (log_window); unneeded embedded in the main window! */ /* delete (log_text_view); unneeded embedded in the main window! */ /* delete (main_notebook); unneeded embedded in the main window! */ /* delete (main_video_image); unneeded embedded in the main window! */ /* delete (local_video_image); unneeded embedded in the main window! */ /* delete (local_video_window); unneeded embedded in the main window! */ /* delete (remote_video_image); unneeded embedded in the main window! */ /* delete (remote_video_window); unneeded embedded in the main window! */ /* delete (video_frame); unneeded embedded in the main window! */ /* delete (pref_window); unneeded embedded in the main window! */ /* delete (ldap_window); unneeded embedded in the main window! */ And so on. Ok let's get serious again now... That kind of comment is non-sense, please remove it. > > 3) + /* the rest of the structure is automatically freed: > > + * the main window has the chat_window, and the chat_window has them > > + */ > > +} > > > > That's not right, how could it be automatically freed? There is a leak > > here. > > You're partly right: I forgot to free the GmTextChat! > Yes, then I'm 100% right as it is what I meant ;-) > But for the rest: when the main window is freed, it frees its children ; > the chat window is one of those. The text view is a child of the chat > window, and the text buffer a child of the text view... > Of course, that is indeed the way GTK works. > > 4) Please don't forget to add forward declarations in C code at the top > > of the file. > > Hmmmm... ok, I'll add them also for the other functions that lack in in > src/chat_window.cpp. > > Snark -- _ Damien Sandras (o- GnomeMeeting: http://www.gnomemeeting.org/ //\ FOSDEM: http://www.fosdem.org v_/_
Attachment:
signature.asc
Description: Ceci est une partie de message =?ISO-8859-1?Q?num=E9riquement?= =?ISO-8859-1?Q?_sign=E9e=2E?=