On Tue, May 24, 2005 at 05:49:09PM -0300, bulia byak wrote:
Before we can think about a release, one thing we absolutely must do is clean up the current mess with dialogs. Everyone who did dialogs recently (and there were a lot) did it in their own incompatible way.
I'm glad this is getting attention.
(Personally, I attribute this to the lack of attention and leadership from the initiators of the gtkmm transition, but there's no use to argue about that now; let's just fix what we have.)
No, much of the mess is due to work done before the gtkmm transition was really initiated. The dialogs that were transitioned into src/ui/dialog/ are all fairly consistent. The one exception being the Preferences dialog, however there had been discussions of redesigning it anyway.
It is true that the last two weeks I have not been putting attention into it, but I think you will find that due to the months I had put into it leading up to this, the project is in a much better position to establish a full solution than it ever was before.
The cause of the shift in attention is probably now obvious to those who follow slashdot. My company had a round of layoffs two weeks ago, and while I wasn't affected, I had to reprioritize my commitments. Anyway, I'm sorry this happened, and apologize if my lack of attention has forced people to scramble. However, the good news is that finally more people are getting seriously involved in this work.
Recently I've been working on the Dialog and DialogManager classes in ui/dialog, and I almost fixed them with regard to things like transientizing, remembering geometry, and F12 behavior. There are remaining issues still but I'm pretty sure I'll be able to fix them.
Thanks, I'm glad you took care of completing this. I had never really understood how all this stuff should work, and had been hoping you could put some effort into it. I hope that you found that with the common Dialog class, it means that this will solve the problem in a permanent way, instead of re-implementing the transient code for each new dialog.
Unfortunately, many dialogs do not use these classes or use them in a wrong way. The old gtk dialogs need not be touched at this time; they are OK as they are, and we can always convert them later. What needs to be fixed urgently is the recenty added gtkmm dialogs, all of which are broken in different ways.
The simplest dialog to use as a template is ui/dialog/messages.cpp; another is ui/dialog/memory.cpp. To create a dialog, you need to pass the prefs path (path in the preferences.xml for the dialog settings; make sure it exists in preferences-skeleton.h) and the verb that creates the dialog (make sure it exists in verbs.h/cpp). E.g.:
Messages::Messages() : Dialog ("dialogs.messages", SP_VERB_DIALOG_DEBUG)
You also need to register the dialog's factory in DIalogManager constructor and to make sure its verb in verbs.cpp does the right thing:
switch (reinterpret_caststd::size_t(data)) { ... case SP_VERB_DIALOG_DEBUG: dt->_dlg_mgr->showDialog("Messages"); break;
Btw, this is intended to only be an interim solution. Ultimately, I would like to see a mechanism that allows the dialogs to be registered in a way that would allow invoking them without passing a string. I.e., instead of a switch statement with cases to handle each verb define as above, you could replace the entire switch statement with just:
dt->_dlg_mgt->showDialog(reinterpret_caststd::size_t(data));
The advantage to this is more than just saving lines of code, though. By doing dialog invokation entirely dynamically, it will enable dynamic addition of dialogs to Inkscape at run time. This in turn means that Ted's dynamically loaded extensions will be able to add their dialogs to Inkscape when they're loaded. My hope is that by allowing extensions to be better decoupled from the core codebase, it will give us better ways of solving extension dependency issues more flexibly.
Also don't forget to remove your own F12 handlers, event handles, transientizing code, title setting, size setting etc. if you have it in your dialog classes. This is all in Dialog now. Not only will this stuff finally work as it must, but this will lead to a significant reducing of code.
Btw, if anyone has a better name for the F12 handler than "f12_handler", PLEASE feel free to rename it. ;-)
Also, I notice that in some cases people are using the 'Impl Trick' such as is shown in memory.*. I had demonstrated this trick a few months ago, and I still feel it is an eligant way to solve header inclusion issues, but the design of Dialog::Manager makes this completely unnecessary. Notice in verbs.cpp and other places that use dialogs, that with the new design the verb handler doesn't need to actually "know" what the dialog is, but just calls routines in the parent Dialog class. This means that the individual dialog headers do NOT need to be included in verbs.cpp or any other places that launch dialogs; thus, the impact of having the private gtkmm widget members defined in the .h is not propagated beyond Dialog::Manager.
Of course, there's nothing *wrong* with using the Impl trick, but I found that it conceptually simplifies the dialog code to not use it, which I feel means the dialog code will be easier for others to understand and maintain.
Thus, I would encourage using the messages.* dialog as the template to follow. This is Ishmal's old Debug dialog, that I converted into the new format. Note that I renamed it from Debug to Messages in order to keep it consistent with what it is called in the Inkscape menus. I notice that in the old dialogs there are major discrepancies between what the dialog is called within the application and what it is called in the code; this leads to unnecessary confusion. If you wish to rename the dialog inside the application, please also rename the file, so when people need to debug a dialog's behavior, it will be extremely obvious which file it is. ;-)
Also, there is a naming convention with the new dialog design, that jonadab and I tried to adhere to. First, do not include 'Dialog' in the dialog's name; it's redundant, and since the class is in the "Inkscape::UI::Dialog::" namespace already, it is unnecessary and just makes the name longer. ;-) Second, use '-' to separate words, so it should be 'clone-tiler.*' not 'clonetiler.*'. Third, make the name as close to the in-Inkscape name, as mentioned above.
Oh, and be attentive to how the name of the dialog shows up in various places. Remember that in Inkscape it shows up in the verb (i.e., the menu and statusbar) as well as in the title of the dialog itself. In some cases in the old dialogs I noticed this got to be inconsistent; it will look a lot better to users if it is consistent throughout.
Bryce
----- End forwarded message -----