Add dependency-injection support to LayoutRegistry

[CamilleLetavernier]

Modify the LayoutRegistry to support LayoutRegistrations via dependency-injection, similar to how it's done for other registries

[gitpod-io]

[CamilleLetavernier]

I'm not sure how the default layouts should be registered. Ideally, we want to be able to remove them, or to associate a different implementation with the same key (e.g. providing our own implementation for the "vbox" layout).

[spoenemann]

Hi @CamilleLetavernier, thanks for the contribution!

You can look at the action handler registrations to see how to define the registration in a way that the default layouts can be overridden:
https://github.com/eclipse/sprotty/blob/6b6f109246bfa56ca11c1a72c0da920d1e194bab/src/base/actions/action-handler.ts#L73-L87

The utility function binds the constructor to itself, so users can override that by rebinding the constructor.

[tortmayr]

Hi @CamilleLetavernier, thanks for the contribution!

You can look at the action handler registrations to see how to define the registration in a way that the default layouts can be overridden:
https://github.com/eclipse/sprotty/blob/6b6f109246bfa56ca11c1a72c0da920d1e194bab/src/base/actions/action-handler.ts#L73-L87

The utility function binds the constructor to itself, so users can override that by rebinding the construc

@spoenemann Maybe we should apply this pattern to all classes that are multiinjected. I haven't had a detailed look yet, but I know that there at least some multiinjected classes that are not bound to itself before they are bound to the multiinjected key.
e.g. https://github.com/eclipse/sprotty/blob/6b6f109246bfa56ca11c1a72c0da920d1e194bab/src/features/move/di.config.ts#L23

[spoenemann]

Yes, that makes sense! In that case it can be expressed more simply:

bind(MoveMouseListener).toSelf().inSingletonScope();
bind(TYPES.MouseListener).toService(MoveMouseListener); 

[CamilleLetavernier]

Thanks for the reviews!

I've addressed the simple comments (Using logger & sorting the TYPES). I also noticed that the tests were broken, because manually instantiating a LayoutRegistry no longer registers the 3 default layouts; it should be fixed now.

I still need to add a "configureLayout" function, similar to the Action handlers, but I won't have time to do that (and test that with my use cases) properly this week; so I'll keep it for later :)

[CamilleLetavernier]

I've rebased the branch on the current master, and added a configureLayout() function

[spoenemann]

Great!

[spoenemann]

Maybe we should apply this pattern to all classes that are multiinjected. I haven't had a detailed look yet, but I know that there at least some multiinjected classes that are not bound to itself before they are bound to the multiinjected key.

@tortmayr is there already an issue about this?

[tortmayr]

Maybe we should apply this pattern to all classes that are multiinjected. I haven't had a detailed look yet, but I know that there at least some multiinjected classes that are not bound to itself before they are bound to the multiinjected key.

@tortmayr is there already an issue about this?

I don't think so. I created #239 to track this.