Re: Temporary tools

[Bernhard Herzog]

Valentin Ungureanu <address@hidden> writes:

> A new patch:
> http://savannah.nongnu.org/patch/?func=detailitem&item_id=4516

I finally got around to this.  Actually, I had a look at the patch a few
weeks ago, but I couldn't make up my mind about some design aspects
involved in temporary tools and then never found the time to do
something about it :-(.  I'm still undecided but I think this should at
least be discussed here.  Note that any problems I have with the patch
are ultimately design problems in Skencil which the patch understandably
doesn't attempt to address.

> A tool change begins with the application context which issues a
> TOOL message. To this message I added a flag which indicates to
> interested subscribers (only the editor so far) that an old tool
> might be restored.
>
> A tool (which is derived from ToolInstance) is temporary if:
>
>     1. has the temporary attribute set to True. This informs the
>        editor to instantiate it without ending the previous tool.
>
>     2. one of its ButtonClick or ButtonRelease methods calls
>        application.SetTool with the previous toolname and the
>        use_old set to True (the tool can peek at some editor
>        attributes like the previous toolname or the handles
>        drawn by the previous tool because the editor passes
>        itself as an argument rather early in the changing tool
>        process).

I think it shouldn't be the responsibility of the tool to restore the
previous tool.  At most it should indicate that it's done, but it
shouldn't have to know which tool to restore and how to do that.
Keeping track of the stack of tools is the responsibility of the editor
or the global context object.

I can think of two approaches to make it better:

 1. When the temporary tool is finished, it calls a new editor method
    called e.g. TemporaryToolFinished (not sure about the name), which
    doesn't take any parameters and restores the previous tool.

 2. Limit temporary tools to a single "mouse-press-drag-release" cycle.
    In the ButtonRelease and ButtonClick methods, the document editor
    (not the tool!) would check whether the current tool is temporary
    and revert to the previous tool.

The main question to decide between these two approaches is: Are there
any temporary tools that need more than one mouse-press-drag-release
cycle?  I can't think of any.  But even if there are none, approach
1. is probably the better one because it uses the strengths of object
orientation better.

However, this still leaves one bigger design problem.  Currently the
current tool is a global setting managed by the context object.  When
activating a temporary tool all editors switch to the temporary tool.
The user uses the temporary tool only in one editor because such a tool
normally only requires one mouse-press-drag-release cycle, but when the
tool is finished all editors have to revert back to the previous tool,
so switching back has to be done via the context object.

The patch does that by calling its SetTool method with the name of the
previous tool.  I think it would be better to have the context keep
track of the stack of tools and give the context a method to end the
current temporary tool (which is what the editor's TemporaryToolFinished
method would call).  The context would then notify all editors that the
temporary tools has finished so that they can revert to the previous
tool object.

What seems wrong about this is that the context *and* the editors have
to keep track of the stack of tools.  That makes two places where this
information is kept and they have to be kept in sync.  Of course, we
have that design problem already with only one permanent tool, but with
temporary tools it gets worse.  

I wonder whether we should get rid of the global tool and simply manage
the current tool at the editor level only.  This might introduce some
usability problems, though, especially with the current global toolbox
window.  Of couse we should get rid of that, too, and give each document
window a menu and toolbar.

Any thoughts?

   Bernhard

-- 
Intevation GmbH                                 http://intevation.de/
Skencil                                           http://skencil.org/
Thuban                                  http://thuban.intevation.org/