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

#418#420 Consume server DI rework #45

Merged
merged 3 commits into from
Nov 5, 2021
Merged

#418#420 Consume server DI rework #45

merged 3 commits into from
Nov 5, 2021

Conversation

tortmayr
Copy link
Contributor

Server

  • Update dependencies to consume latest DI rework
  • Update .tpd file and rename it to r2021-03
  • Adapt existing server code to conform to latest API changes
  • Align prefix for eclipse-integration specific classes. (Use Ide over Eclipse)
  • Update .settings/org.eclipse.jdt.ui.prefs for glsp.ide.editor to ensure that the copyright header for new files has the correct year.
  • Introduce a wrapper GLSPClient implementation (IdeGLSPClient) that handles dedicated GLSPClient proxies for each client session and can be connected to the GLSPServer (#420)
  • GLSPDiagramEditor:
    -- Refactor Browser-URL generation
    -- Generate applicationID and send to client via url query
  • Add command to DiagramMenu to open the current DiagramWidget in an external browser for debugging purposes.

Client:

  • Update dependencies to latest glsp nightlies
  • Adapt code to conform to latest protocol changes
  • [WF-Example] Retrieve application id from url params
  • [WF-Example] Update webpack config to webpack 4 and ensure proper source-map generation for debugging
  • [WF-Example] Fix webpack build to support codicons (#406)

Fixes eclipse-glsp/glsp/issues/418
Fixes eclipse-glsp/glsp/issues/420
Part of eclipse-glsp/glsp/issues/406

Server
- Update  dependencies to consume latest DI rework
- Update .tpd file and rename it to `r2021-03`
- Adapt existing server code to conform to latest API changes
- Align prefix for eclipse-integration specific classes. (Use `Ide` over `Eclipse`)
- Update  .settings/org.eclipse.jdt.ui.prefs for glsp.ide.editor to ensure that the copyright header for new files has the correct year.
- Introduce a wrapper `GLSPClient` implementation (`IdeGLSPClient`) that handles dedicated GLSPClient proxies for each client session and can be  connected to the `GLSPServer` (#420)
- GLSPDiagramEditor:
-- Refactor Browser-URL generation
-- Generate applicationID and send to client via url query
- Add command to DiagramMenu to open the current DiagramWidget in an external browser for debugging purposes.

Client:
- Update dependencies to latest glsp nightlies
- Adapt code to conform to latest protocol changes
- [WF-Example] Retrieve application id from url params
- [WF-Example] Update webpack config to  webpack 4 and ensure proper source-map generation for debugging
- [WF-Example] Fix webpack build to support codicons (#406)

Fixes eclipse-glsp/glsp/issues/418
Fixes eclipse-glsp/glsp/issues/420
Part of eclipse-glsp/glsp/issues/406
@tortmayr
Copy link
Contributor Author

FYI: This PR also improves the debugging experience for frontend code:

  • Webpack config has been updates so that TS-source maps a properly supported in the browser-debugger
  • If the same diagram is opened twice (editor & external browser) changes are synchronized (thanks to GLSPServer should be able to handle multiple GLSPClient proxies glsp#420)
  • A "Debug (External Browser)" command has been added to the diagram menu, which opens the currently open diagram in the external browser.

@tortmayr
Copy link
Contributor Author

Notes for testing: This PR contains client changes so a client-rebuild is necessary.

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.

Thank you very much Tobias for that update! Everything works and looks great! I especially like the Debug in External Browser action and that the browser and the Eclipse IDE are properly synced, that should make our lives much easier :D

I just had one little issue with the yarn.lock you submitted, could you please have a look at it to double-check whether it is not just on my end, thank you!

client/yarn.lock Outdated
resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.8.tgz#cd924bf5200a075b83c188cd6b9e211b7fc0d3ed"
integrity sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==
version "1.3.9"
resolved "https://registry.yarnpkg.com/rc/-/rc-1.3.9.tgz#e59af47fc44a766bbb7c6536df7adce2722e9421"
Copy link
Contributor

Choose a reason for hiding this comment

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

This version is not available for me, instead I need to use 1.2.8:

  version "1.2.8"
  resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.8.tgz#cd924bf5200a075b83c188cd6b9e211b7fc0d3ed"
  integrity sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==

This can also be easily tested if you just enter https://registry.yarnpkg.com/rc/-/rc-1.3.9.tgz in your browser, it responds with 'Not found'. Could you please double check and push the new yarn.lock if you have the same problem.

queryParams.put("path", path);
queryParams.put("port", "" + manager.getLocalPort());
queryParams.put("widget", getWidgetId());
queryParams.put("application", APPLICATION_ID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can have a dedicated protected method to overwrite/add additional parameters without having to override the complete method?

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.

Fantastic, thank you for reacting so quickly! LGTM!

@tortmayr tortmayr merged commit d43f740 into master Nov 5, 2021
@tortmayr tortmayr deleted the tortmayr/418 branch November 5, 2021 11:58
@ivy-cst
Copy link
Contributor

ivy-cst commented Nov 5, 2021

@tortmayr Thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants