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

Version Updates #162

Merged
merged 6 commits into from
Jan 10, 2024
Merged

Version Updates #162

merged 6 commits into from
Jan 10, 2024

Conversation

NiklasRentzCAU
Copy link
Member

Updates Sprotty to version 1.1.0 and alongside it now requires node 16, es2017, new webpack 5 and others.

@NiklasRentzCAU NiklasRentzCAU added the enhancement New feature or request label Jan 9, 2024
Copy link
Contributor

@Eddykasp Eddykasp left a comment

Choose a reason for hiding this comment

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

Quote mark formatting could be done in a separate dedicated PR, it's still a little unclear what part of the messaging is now broken and/or untested.

Is there anything else besides the bookmarks that is now disabled or no longer working?

},

node: {
net: "mock",
__dirname: 'mock',
Copy link
Contributor

Choose a reason for hiding this comment

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

inconsistent use of ' and " in this file

Copy link
Member Author

Choose a reason for hiding this comment

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

Created #163 to fix this and the other formatting comments in one sweep in another PR.

```

### Dispatching Actions
*This feature is currently not supported*
Copy link
Contributor

Choose a reason for hiding this comment

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

does this break any of our stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

This feature has not been used in any other extension, so it does not break anything either.

}
})
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline at end of file


extension.webviews.forEach((webview) => webview.dispatch(action));
// TODO: this has not been tested yet if the messenging change works like this.
messenger.sendNotification({ method: "diagram/accept" }, { type: 'webview', webviewType: diagramType }, action);
Copy link
Contributor

Choose a reason for hiding this comment

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

what exactly is affected by this and what needs to be done to test whether this works?

Copy link
Member Author

Choose a reason for hiding this comment

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

// Command provided for other extensions to dispatch an action if a webview is open
As this comment above suggests, this is the feature that I marked as currently not supported in the documentation. To test this we would need to use this feature in another extension, i.e. dispatch some action that should be executed from that other extension. As I don't see a use case currently, I think this is fine to document it being (potentially) not fully supported atm.

*
* SPDX-License-Identifier: EPL-2.0
*/
import "reflect-metadata";
Copy link
Contributor

Choose a reason for hiding this comment

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

inconsistent use of ' and " in this file

await super.receiveAction(message);
await this.languageClient.sendNotification(acceptMessageType, message);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

super.didCloseWebview(endpoint);
this.storageService.setItem('diagramOpen', false)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

return this.root
}

redo(_context: CommandExecutionContext): SModelRoot {
redo(_context: CommandExecutionContext): SModelRootImpl {
return this.root
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

@NiklasRentzCAU NiklasRentzCAU merged commit c9365de into main Jan 10, 2024
1 check passed
@NiklasRentzCAU NiklasRentzCAU deleted the nre/versions branch January 10, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants