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

Ensure we do not return an invalid anchor point during edge routing #325

Merged
merged 2 commits into from Mar 10, 2024

Conversation

martin-fleck-at
Copy link
Contributor

What it does

Fixes eclipse-glsp/glsp#1270

Basically the problem is that the DiamondAnchor may return an invalid point during its calculation if you give it an element which has as center the given refPoint, resulting in a 0-length PointToPointLine and ultimately to a division by zero.

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)

import { injectable } from 'inversify';

@injectable()
export class GLSPPolylineEdgeRouter extends PolylineEdgeRouter {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I recall correctly we decided in a previous discussion regarding the naming of the feature modules to not use the GLSP prefix if possible and rather rexport the parent sprotty class with a sprotty prefix.

So we should probably change this to

Suggested change
export class GLSPPolylineEdgeRouter extends PolylineEdgeRouter {
export class PolylineEdgeRouter extends SprottyPolylineEdgeRouter {

and add the corresponding rexport to the glsp-sprotty package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I pushed an update that properly deals with the sprotty-fixed versions and the re-export!

Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

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

Thanks for the update. LGTM!

@martin-fleck-at martin-fleck-at merged commit c4764a5 into master Mar 10, 2024
6 checks passed
@martin-fleck-at martin-fleck-at deleted the issues/1270 branch March 10, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants