Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GLSP-849: Remove theia-glsp-connector #173

Merged
merged 2 commits into from
Aug 8, 2023
Merged

GLSP-849: Remove theia-glsp-connector #173

merged 2 commits into from
Aug 8, 2023

Conversation

tortmayr
Copy link
Contributor

Switch to new GLSPModelSource Instead of TheiaGLSPDiagramServer´. With this migration we also remove the concept of the TheiaGLSPConnector`. It is no longe needed since all diagram containers are child containers of the Theia main container and have direct access to all Theia services.

Refactors theia specifc GLSP customiizations/extensions into feature modules similar to how we handle it for the standalone usecase. This appraoch is now possible because the diagram container is a real child container of the Theia main container which means we have access to all Theia services. This includes

  • Refactoring/Renaming of the SaveableGLSPModelSource toGLSPSaveable. The saveable implementation listens to the onDirtyStateChanged event from the diagram EditorContextService
  • Move Theia export svg functionality from connector to dedicated theiaExportModule
  • Move forwarding to Theia selection service into dedicated theiaSelectModule
  • Move the Theia source model changed handler into dediacted theiaSourceModelWatcherModule
  • Move handling of external navigation targets into dedicated theiaNavigationModule
  • Load the Theia default feature modules in the GLSPTheiaDiagramConfiguration
  • Move server message & process handling into dedicated notificationModule -- Remove GlspNotificationManager customization as its no longer needed

Another advantage of moving those bindings directly into the child contain is the multi-editor support (i.e. multiple registered GLSP editors in one instance). Previously lots of the services (like TheiaSourceModelChangedHandler) would be shared between editor implementations. Now each implementation maintains its own set of services and can customize/rebind them as needed.

Fixes eclipse-glsp/glsp#849 Part of eclipse-glsp/glsp#900 Part of eclipse-glsp/glsp/issues/850

Requires eclipse-glsp/glsp-client#272

Switch to new `GLSPModelSource` Instead of `TheiaGLSPDiagramServer´. With this migration we also remove the concept of
the `TheiaGLSPConnector`. It is no longe needed since all diagram containers are child containers  of the Theia main container and have direct access to all Theia services.

Refactors theia specifc GLSP customiizations/extensions into feature modules similar to how we handle it for the standalone usecase. This appraoch is now possible because the diagram container is a real child container of the Theia main container which means we have access to all Theia services.
This includes
 - Refactoring/Renaming of the `SaveableGLSPModelSource` to`GLSPSaveable`. The saveable implementation listens to the `onDirtyStateChanged` event from the  diagram `EditorContextService`
- Move Theia export svg functionality from connector to dedicated `theiaExportModule`
- Move forwarding to Theia selection service into dedicated `theiaSelectModule`
- Move the Theia source model changed handler into dediacted `theiaSourceModelWatcherModule`
- Move  handling of external navigation targets into dedicated `theiaNavigationModule`
- Load the Theia default feature modules in the `GLSPTheiaDiagramConfiguration`
- Move server message & process handling into dedicated `notificationModule`
  -- Remove `GlspNotificationManager` customization as its no longer needed

Another advantage of moving those bindings directly into the child contain is the multi-editor support (i.e. multiple registered GLSP  editors in one instance). Previously lots of the services (like TheiaSourceModelChangedHandler) would be shared between editor implementations. Now each implementation maintains its own set of services and can customize/rebind them as needed.

Fixes eclipse-glsp/glsp#849
Part of eclipse-glsp/glsp#900
Part of eclipse-glsp/glsp/issues/850

Requires eclipse-glsp/glsp-client#272
Copy link
Member

@planger planger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent change, less code but same functionality :-)
Just one minor rename comment below.

theiaExportModule,
theiaNavigationModule,
theiaSourceModelWatcherModule,
notificationModule
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should rename that module to theiaNotificationModule for consistency.

@tortmayr tortmayr requested a review from planger August 8, 2023 09:17
Copy link
Member

@planger planger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me! 👍

@tortmayr tortmayr merged commit 591932b into master Aug 8, 2023
@tortmayr tortmayr deleted the glsp-849 branch August 8, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove theia-glsp-connector
2 participants