Some problems with the current plugin system in gnucash GUI

[Josh Sled]

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 ${a}@${b}`