Switching shapes and tools: is it broken?

[Dag Andersen]

Before the weekend I fixed a crash in ArtisticTextShape and, as Jan Hambrect 
pointed out, I unvittingly removed the functionallity to go direct between 
shapes. So I thought I'd find out how this *should* be implemented before I 
tried to put it back, but I can't!
I can find code that afaics is supposed to do it, but it is never executed. It 
may be that it is just me being confused so I need to run this by somebody 
that is more familiar with the code than I am.

There is two "types" of switches that can occure:
1) Switch between shapes with different tools.
E.g. TextTool -> ArtisticTextTool.
2) Switch between shapes using the same tool.
E.g. TextTool -> TextTool

From a users pov I'd say the two switches should be identical (unless maybe if 
there is some kind of connection between the shapes).
From a developers pov I'd expect this to have a default implementation that I 
can reimplemt if I have any specific requirements.
None of the above holds.
Testing different shapes shows that some implement both types of switches, some 
one of them and some none.

Looking at the code shows that type 1) switch is implemented in 
KoToolProxy::mouseReleaseEvent(). The code is executed *if and only if* the 
tool calls event->igonere() in its implentation of mouseReleaseEvent.
Some do, some have missed the trick as it is not well documented.
I propose:
* Implement KoToolBase::mouse*Event handlers (now pure vitual) with a call to 
event->ignore();
* Make the execution of the switching code the default behaviour either by 
making ignore the default, or by having an explicit method for disabling 
switching by KoToolProxy.

Type 2) swithing:
This is now handled by (some of) the individual tools (in slightly different 
ways).
I have found code in KoToolManager::Private::switchTool() though, that seems 
to be implemented to do just this. It is never executed, because there is this 
check:

if (canvasData->activeTool == tool && tool->toolId() != KoInteractionTool_ID)  
	return;

Does anyone know the reason for this check?
Surface testing shows that disabling it works.
-- 
Mvh.
Dag Andersen