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-1438: Improve GLSPClient default implementations #402

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Dec 5, 2024

What it does

  • Introduce onCurrentStateChanged event for GLSPClient
  • Ensure that start and initializeServer behave as expected if they are called again while the previous promise is still pending.
  • Restructure members of the default implementations so that get/setters and their _property are grouped together
  • Introduce a dedicated namespace for the Event interface with utility functions to
    • listen on a certain event exactly once
    • waitUntil a certain event is fired the next time
  • Adapt test cases to verify new behavior

Also:

  • Update GLSPActionDispatcher to use direct injection for registry instead of async provider

How to test

Follow-ups

Changelog

  • This PR should be mentioned in the changelog
  • This PR introduces a breaking change (if yes, provide more details below for the changelog and the migration guide)

- Introduce `onCurrentStateChanged` event for `GLSPClient`
- Ensure that `start` and `initializeServer` behave as expected if they are called 
  again while the previous promise is still pending.
- Restructure members of the default implementations so that get/setters and their _property are grouped together
- Introduce a dedicated namespace for the `Event` interface with utility functions to
  - listen on a certain event exactly once
   - waitUntil a certain event is fired the next time
- Adapt test cases to verify new behavior

Also:
- Update `GLSPActionDispatcher` to use direct injection for registry instead of async provider
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Change looks good to me and everything still seems to work as expected. I just have a question regarding the new Event utility methods but other than that, everything should be good, thank you Tobias!

packages/protocol/src/utils/event.ts Show resolved Hide resolved
packages/protocol/src/utils/event.ts Show resolved Hide resolved
@tortmayr tortmayr merged commit 5a48fb6 into master Dec 6, 2024
7 checks passed
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
+Fix eclipse-glsp#401 Node creation  (eclipse-glsp#402)
+ Resolve eclipse-glsp#398 Refactor TypeHints API (eclipse-glsp#399)
+ Resolve eclipse-glsp#396 RouterKind for FeedbackEdge (eclipse-glsp#397)
+ Reuse Sprotty's request/response system and remove custom solution(eclipse-glsp#404)
+ Add support for context menu and harmonize with command palette (eclipse-glsp#405)
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
+Fix eclipse-glsp#401 Node creation  (eclipse-glsp#402)
+ Resolve eclipse-glsp#398 Refactor TypeHints API (eclipse-glsp#399)
+ Resolve eclipse-glsp#396 RouterKind for FeedbackEdge (eclipse-glsp#397)
+ Reuse Sprotty's request/response system and remove custom solution(eclipse-glsp#404)
+ Add support for context menu and harmonize with command palette (eclipse-glsp#405)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants