Bryce Harrington wrote:

Bob,

I have a few questions on a couple dialogs...

I notice that "verbs.h" is included in the headers. I see this is for the 'verb_num' argument that's been added to the Dialog constructor. I looked in Dialog::Dialog() but it doesn't appear to get used in that function for anything. It would be nice if we didn't include verbs.h in the dialog headers, because verbs.h changes often and would force the dialogs to be recompiled excessively. What is the verb_num argument for?

I have no idea. I think that someone added it later. Feel free to take it out if there is no visible use for it.

Regarding naming, Jizzbug and I felt that the new dialogs should not have 'dialog' in their title, as it's redundant since they're in the Inkscape::UI::Dialog namespace already. Would it be possible to rename TraceDialog to Trace, and ScriptDialog to Script?

This was in order to differentiate the dialog from the object it is controlling. There is an Inkscape::Trace::Trace class that is used by the TraceDialog, and an Script object controlled by the script dialog. But now I guess that the namespaces are good enough.

In TraceDialog, I notice you have a TraceDialogImpl class; it turns out that due to DialogManager, the dialogs are encapsulated well enough that we really don't need to use the Impl trick.

The intention of this separation of interface/impl was to allow external code to use the interface without needing to link to everything in the Impl, thus avoiding the cascading-dependency problem. That would cause an external plugin that wanted to use them to link to a huge amount of unnecessary code, making plugins much too large. However, XPath (still in steady progress, I swear) will take care of this.

Bob