GLSP-1349 Improve container configuration #355
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `replace` property to `ModulConfiguration` interface. When resolving feature modules of the replace property, any potential already configured module with the same id will be replaced with the defined module. If there is nothing to replace, the module will be added to the end of the resolved configuration - Add check for non-unique feature module ids to `resolveContainerConfiguration` function. - Add option to enable additional debug logging when loading feature modules. This is usefull during development to get insights about why the container modules are not loaded as expected - Add dedicated `featureIds` to all default modules. If no featureId is provided the number id of the module is reused for creating the featureId. While this works perfectly fine, it makes debugging harder because feature modules are hard to indentify based on their featureid. With this change its now easier to derive the corresponding feature module from the featureid. Fixes eclipse-glsp/glsp#1349
martin-fleck-at
requested changes
Jun 6, 2024
| @@ -46,5 +46,5 @@ export const standaloneExportModule = new FeatureModule( | |||
| bind(ExportSvgActionHandler).toSelf().inSingletonScope(); | |||
| configureActionHandler(context, ExportSvgAction.KIND, ExportSvgActionHandler); | |||
| }, | |||
| { requires: exportModule } | |||
| { featureId: Symbol('standaloneExport'), requires: exportModule } | |||
There was a problem hiding this comment.
The exportModule a few lines up could also profit from an actual Symbol because currently it just gets a number assigned.
| if (existingIndex >= 0) { | ||
| modules[existingIndex] = replace; | ||
| } else { | ||
| distinctAdd(modules, replace); |
There was a problem hiding this comment.
I know you defined this as the intended behavior in the function comments but I feel that the replace behavior should still log a warning if it does not actually replace another module but defaults to the adding behavior. What do you think?
tortmayr
added a commit
that referenced
this pull request
Jun 18, 2024
- Update `onSelect` method of autocomplete Widget to dispatch a `input` event instead of a `keyup` event. (Since autocompleter 8.x the suggestion update uses `input` instead of `keyup` - Enable task editor in standalone example by adding an explicit standaloneTaskEditorModule. Fixes eclipse-glsp/glsp#1347 In addition: - Update dependencies and align with sprotty - Use fixed version of vscode-jsonrpc to avoid conflicts with Theia - Restore original container configuration order in `workflow-diagram-module` (was changed on accident in #355)
2 tasks
tortmayr
added a commit
that referenced
this pull request
Jun 18, 2024
- Update `onSelect` method of autocomplete Widget to dispatch a `input` event instead of a `keyup` event. (Since autocompleter 8.x the suggestion update uses `input` instead of `keyup` - Enable task editor in standalone example by adding an explicit standaloneTaskEditorModule. Fixes eclipse-glsp/glsp#1347 In addition: - Update dependencies and align with sprotty - Use fixed version of vscode-jsonrpc to avoid conflicts with Theia - Restore original container configuration order in `workflow-diagram-module` (was changed on accident in #355)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it does
replaceproperty toModulConfigurationinterface. When resolving feature modules of the replace property, any potential already configured module with the same id will be replaced with the defined module. If there is nothing to replace, the module will be added to the end of the resolved configurationresolveContainerConfigurationfunction.featureIdsto all default modules. If no featureId is provided the number id of the module is reused for creating the featureId. While this works perfectly fine, it makes debugging harder because feature modules are hard to indentify based on their featureid. With this change its now easier to derive the corresponding feature module from the featureid.Fixes eclipse-glsp/glsp#1349
How to test
resolveContainerConfigurationfunction are inplace and they are also testing the replace behaviordi.configfile of the workflow-standalone example:Follow-ups
Changelog