On Wed, May 25, 2005 at 10:38:59AM -0400, MenTaLguY wrote:
On Wed, 2005-05-25 at 01:23, Bryce Harrington wrote:
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_cast<std::size_t>(data));...
Hmm. I'm not overly fond of this approach.
It seems to be refactoring the wrong pieces to me. I'd like to stick with strings or GQuarks to identify dialogs. There's no reason to tie dialogs to verbs directly. Refactorings should follow abstraction boundaries.
Right, the above is just conceptually what I'm thinking. The idea being to eliminate the switch statement and replace it with a single call that passes some data item to indicate what dialog to launch. Whether that data item is a verb, or a string, or a GQuark, I don't know. You'll note that I added some GQuark support in Dialog::Manager. I sort of suspect that may be the ideal way to go.
Rather than having a special case for verbs that spawn dialogs, it seems a better idea to provide the ability for a dynamically loaded extension to register a general-purpose callback as a verb. Remember, a verb that shows a dialog might well want to do some preparatory work on the document too.
Ah, good idea. Of course, that's beyond the scope of dialogs.
Btw, gtkmm has its own entirely new idea of how to handle verbs (called 'Actions'), that is fairly incompatible with what is in verbs.*. Jonadab and I did most of the work to create that code, but it's quite different than how things are done in the current codebase. The dialogs system is not dependent on it, so it won't be an issue for a while, but I think you should take a long look at it.
My feeling is that the gtkmm Action system has features that our current verb system lacks. I think it may make it possible to do verbs more dynamically than currently. On the other hand, it feels a bit overly verbose; it seems adding a new menu item requires adding the same thing in two or three places, and I've wondered if there might be a better way to do that. It uses an XML file to connect actions to menu items, which is sort of convenient (no compiling!) but is a little awkward to troubleshoot sometimes - in a few cases I've mismatched the action between the code and the xml, such as plural in one place and single in the other, and you kind of need to know where all to look in order to fix it. Anyway, maybe this is just a minor quibble, so I think you should look at it and see what you think.
I would suggest that if we feel the current verbs system has advantages over the gtkmm Action system, that rather than avoiding use of Actions, that some consideration be put into helping improve Actions, since it seems to be the standard way for handlign menu events for Gtkmm apps.
Also, showDialog("dialogName") doesn't seem like an inordinate amount of code. I'd be more concerned about eliminating the cut-and-pasting of dt->_dlg_mgr and the associated exposure of internal details.
Correct. This was kind of a shortcut to getting the new dialog code integrated. I had thought that perhaps this could be done by adding a member routine to SPDesktop for launching dialogs. However, I opted to leave it exposed until that other issue was sorted out, regarding how dialogs should be shown.
Btw, if you look at the verb handlers for some of the other dialogs, some do more than merely show the dialog; some do some extra prep work. I haven't looked very deeply at that, but I suspect it may impose requirements that may make it inconvenient to just have a showDialog() routine. It may be necessary to have a getDialog() routine, to allow the verb handler a chance to make those customizations. However, I think a preferable solution would be to push that custom stuff down into the dialog's code itself, so all dialogs can be handled the same way. Again though, I haven't looked at those things at all, so I don't know if that's easy or hard to do.
Bryce