Re: Temporary tools

[Valentin Ungureanu]

Bernhard Herzog wrote:
> Valentin Ungureanu writes:
> 
> >> I wonder whether we should get rid of the global tool and simply
> >> manage the current tool at the editor level only.
> [...]
> > The context would be used only to pass a SetTool call to the editor,
> > and it would be the task of the editor to send a TOOL message after
> > it sets the new tool.  To this message would be subscribed the
> > canvas (to set a new cursor), some menu items and the tool buttons.
> 
> Indeed. That's what had to be done to implement it.  See the new patch
> http://savannah.nongnu.org/patch/?func=detailitem&item_id=4695
> I'd like to get some feedback on this before I commit it.
> 

Two small problems:
1. Starting Skencil, choosing File, New and then closing the editor
   window gives an AttributeError.

2. Start Skencil and press some tool buttons; they remain depressed.


> > We could subscribe the tool buttons to the context.  This way each
> > button can adjust its sensitivity and set itself as active or not
> > when the context changes.  So even if the toolbox doesn't go away it
> > would always reflect the tool of the current editor.
> 
> That's basically the way it was already implemented even without the
> new patch.  Whether a button is active or not is effectively a view
> (in MFC sense) on the current tool.
      ^^^ freudian slip? :)

I think the model should be the editor's tool.
If there is no editor active then there is no model to have a view for,
so maybe the toolbox should be insensitive when this is the case.

I also started a patch about two weeks ago.
The most notable differences where that context.tool was a property (and
assigning to context.tool called context.SetTool) and that the TOOL message
didn't carry any arguments, the subscribers checked context.tool instead.
Also if there was no editor context.tool was None and not 'SelectionTool'.

For now, I made a patch against current CVS + your patch:

ISTM that SketchApplication.SetTool and SketchCanvas.SetTool are not needed
anymore so I removed them.

Also I modified ToolButton to be insensitive when there is no editor (which
makes them look garbled, but I suppose this can be fixed).


Straying away a bit: I made some experiments with removing ToolInfo.

I suppose the initial idea was to have ToolInstance manage only the
interaction with the editor and having ToolInfo provide the rest of necessary
informations (icon, etc).  I wonder if this is not more complex than it needs
to be, especially now with moving the tools "closer" to the editor.

The same functionality can be provided by adding only three class attributes
to ToolInstance: name, cursor and active_cursor.  If we name this new class
"Tool" instead of "ToolInstance" the we will have, well, a tool :)

The icon filename would be the name of the tool (which in ToolInfo is the
current fallback anyway).  Arguably, "editor.tool.name" makes more sense than
"editor.toolname".

Now, to implement a new tool one needs to write a class derived from
ToolInstance, one derived from ToolInfo and to add the latter to
Sketch.Editor.toolmap.

Registering a class might be a reasonable use of metaclasses, so with a very
simple metaclass this could be reduced to writing a class derived from Tool.

Also, writing advanced scripts for Skencil might be simplified a bit.

The patch is almost finished, I'll submit it when it's done (unless you think
this is not worth pursuing at all)

Valentin