On Sat, 2005-11-12 at 13:54 +0100, Didier Vidal wrote: > However, I have a few problems with this architecture: > - The GUI actions (in this case, open the account hierarchy druid, > open a new account tree page) are managed by a class in the engine > module. That's odd to me. engine should be about data and data > consistency, not about user interactions.
I don't think it's unreasonable for a library to provide a hook mechanism used by the calling layers. The engine is also partially about pieces of the generic application... in particular, here, this is just the generic "new book" hook. It says nothing about a particular user-interaction paradigm, and it's not managing those interactions. It's just providing a way for application-code to be informed and do something when that event occurs. The engine is the best place to know when new books are created. We could think about seperating the core accounting data engine from the core accounting-application engine... but I think we'd end with something pretty similar to what we have now. > - The order of events is not controlled (you don't know in which order > the hooks will be called) Yeah... a generic list of hooks with inability to control the execution order is of limited utility. > - The GUI logic is not defined in a central place. To understand what > happens, you have to grep the hooks in all the source code. > - The GUI logic is also limited. For instance, you can't abort a > sequence if one of the actions (the hooks) is canceled by the user. The opposite arguments hold here, though: - the GUI logic can be extended by modules and extensions without needing to be defined in a central place. - any of those extensions has a chance to modify execution flow to do what it needs to. At the same time, I also find it pretty annoying that so many things are so indirectly specified. It makes for a pretty inscrutable codebase. > Did I use the right way to implement the feature ? Hmm. With this patch, the hook is registered at account-tree plugin-page class-initialization time, and the hook applies to all new books, regardless of origin. That doesn't seem quite right on either count... The rule here is that we should create an account-tree page when a main-window is created which doesn't otherwise have an account-tree page. You want to hook in [:)] at a point after the UI is created, but you don't want to create an account-tree page *if* the saved-window-state contains one already, and you end up with two in the window. And there's no HOOK_NEW_MAIN_WINDOW_CREATED_WITHOUT_SAVED_STATE_ALREADY_WITH_AN_OPEN_ACCOUNT_TREE_PAGE. :) Ideally, you'd want to define this rule outside of the main-window code, as the main-window code doesn't presently know much about these application semantics, and that's a good thing. Practically the only time the NEW_BOOK hook is run is in gnc-file.c:gnc_file_new(), right after the book is created. That's invoked by the New File menu. I guess you could add the code directly to the end of gnc_file_new(), instead of indirecting through the hook. That's more clear, I think. If we create some other mechanism for creating a new book+main-window, however, we'll need to do the same thing there. But we can refactor as appropriate then. > If yes, aren't we a > little to extreme with the concept of plugins in Gnucash ? Sometimes. In particular in this case, while all the main-window UI pieces are called "plugin-pages", they're not necessarily plugins in the traditional sense. We definitely want to have an architecture that lets optional functionality integrate with the system ... e.g. a inventory subsystem that registers a new plugin-page and menu/toolbar-additions when its module is loaded at runtime. And if we have that, then everything should be implemented in that manner if only for consistency. But nothing prevents us from saying that -- at least in the context of the GTK front-end -- the account-tree plugin page is part of the "required set", will always be available, and as such we can hard-code for it in the code invoked by the GTK front-end menu item. There's no need to indirect through the hooks. ...jsled -- http://asynchronous.org/ - `a=jsled; b=asynchronous.org; echo [EMAIL PROTECTED] _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
