From 75f12c62ae274bba4c9bf5f70e874d72f71c4b3d Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Tue, 27 Feb 2024 10:30:02 +0100
Subject: [PATCH 1/7] Ensure we apply feedback actions already on SetModel

Fixes https://github.com/eclipse-glsp/glsp/issues/1239
---
 packages/client/src/base/default.module.ts    |  3 +
 packages/client/src/base/feedback/index.ts    |  2 +
 .../src/base/feedback/set-model-command.ts    | 79 +++++++++++++++++++
 .../src/base/feedback/update-model-command.ts |  6 +-
 4 files changed, 87 insertions(+), 3 deletions(-)
 create mode 100644 packages/client/src/base/feedback/set-model-command.ts

diff --git a/packages/client/src/base/default.module.ts b/packages/client/src/base/default.module.ts
index 6690f866..0fa2178f 100644
--- a/packages/client/src/base/default.module.ts
+++ b/packages/client/src/base/default.module.ts
@@ -22,6 +22,7 @@ import {
     MoveCommand,
     SetDirtyStateAction,
     SetEditModeAction,
+    SetModelCommand,
     TYPES,
     bindAsService,
     bindOrRebind,
@@ -35,6 +36,7 @@ import { GLSPActionDispatcher } from './action-dispatcher';
 import { GLSPActionHandlerRegistry } from './action-handler-registry';
 import { GLSPCommandStack } from './command-stack';
 import { EditorContextService } from './editor-context-service';
+import { FeedbackAwareSetModelCommand } from './feedback';
 import { ModifyCssFeedbackCommand } from './feedback/css-feedback';
 import { FeedbackActionDispatcher } from './feedback/feedback-action-dispatcher';
 import { FeedbackAwareUpdateModelCommand } from './feedback/update-model-command';
@@ -73,6 +75,7 @@ export const defaultModule = new FeatureModule((bind, unbind, isBound, rebind, .
     // Model update initialization ------------------------------------
     bind(TYPES.IFeedbackActionDispatcher).to(FeedbackActionDispatcher).inSingletonScope();
     configureCommand(context, FeedbackAwareUpdateModelCommand);
+    rebind(SetModelCommand).to(FeedbackAwareSetModelCommand);
 
     bind(GLSPMouseTool).toSelf().inSingletonScope();
     bindOrRebind(context, MouseTool).toService(GLSPMouseTool);
diff --git a/packages/client/src/base/feedback/index.ts b/packages/client/src/base/feedback/index.ts
index 9a753e06..0ba61ca6 100644
--- a/packages/client/src/base/feedback/index.ts
+++ b/packages/client/src/base/feedback/index.ts
@@ -16,4 +16,6 @@
 export * from './css-feedback';
 export * from './feedback-action-dispatcher';
 export * from './feedback-command';
+export * from './set-model-command';
 export * from './update-model-command';
+
diff --git a/packages/client/src/base/feedback/set-model-command.ts b/packages/client/src/base/feedback/set-model-command.ts
new file mode 100644
index 00000000..ef680272
--- /dev/null
+++ b/packages/client/src/base/feedback/set-model-command.ts
@@ -0,0 +1,79 @@
+/********************************************************************************
+ * Copyright (c) 2024 EclipseSource and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+import {
+    ActionHandlerRegistry, Command, CommandActionHandler, CommandExecutionContext, GModelRoot, ILogger, SetModelAction,
+    SetModelCommand, TYPES, toTypeGuard
+} from '@eclipse-glsp/sprotty';
+import { inject, injectable, optional } from 'inversify';
+import { IFeedbackActionDispatcher } from './feedback-action-dispatcher';
+import { FeedbackCommand } from './feedback-command';
+
+@injectable()
+export class FeedbackAwareSetModelCommand extends SetModelCommand {
+    @inject(TYPES.ILogger)
+    protected logger: ILogger;
+
+    @inject(TYPES.IFeedbackActionDispatcher)
+    @optional()
+    protected feedbackActionDispatcher: IFeedbackActionDispatcher;
+
+    protected actionHandlerRegistry?: ActionHandlerRegistry;
+
+    constructor(
+        @inject(TYPES.Action) action: SetModelAction,
+        @inject(TYPES.ActionHandlerRegistryProvider)
+        actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>
+    ) {
+        super(action);
+        actionHandlerRegistryProvider().then(registry => (this.actionHandlerRegistry = registry));
+    }
+
+    override execute(context: CommandExecutionContext): GModelRoot {
+        const root = super.execute(context);
+        this.applyFeedback(root, context);
+        return root;
+    }
+
+    protected applyFeedback(newRoot: GModelRoot, context: CommandExecutionContext): void {
+        if (this.feedbackActionDispatcher && this.actionHandlerRegistry) {
+            // Create a temporary context which defines the `newRoot` as `root`
+            // This way we do not corrupt the redo/undo behavior of the super class
+            const tempContext: CommandExecutionContext = {
+                ...context,
+                root: newRoot
+            };
+            const feedbackCommands = this.getFeedbackCommands(this.actionHandlerRegistry);
+            feedbackCommands.forEach(command => command.execute(tempContext));
+        }
+    }
+
+    protected getFeedbackCommands(registry: ActionHandlerRegistry): Command[] {
+        const result: Command[] = [];
+        this.feedbackActionDispatcher.getRegisteredFeedback().forEach(action => {
+            const commands = registry
+                .get(action.kind)
+                .filter<CommandActionHandler>(toTypeGuard(CommandActionHandler))
+                .map(handler => handler.handle(action));
+            result.push(...commands);
+        });
+        // sort commands descending by priority
+        return result.sort((a, b) => this.getPriority(b) - this.getPriority(a));
+    }
+
+    protected getPriority(command: Partial<FeedbackCommand>): number {
+        return command.priority ?? 0;
+    }
+}
diff --git a/packages/client/src/base/feedback/update-model-command.ts b/packages/client/src/base/feedback/update-model-command.ts
index 18a8f05d..cebab5e7 100644
--- a/packages/client/src/base/feedback/update-model-command.ts
+++ b/packages/client/src/base/feedback/update-model-command.ts
@@ -13,7 +13,6 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { inject, injectable, optional } from 'inversify';
 import {
     ActionHandlerRegistry,
     Animation,
@@ -21,15 +20,16 @@ import {
     CommandActionHandler,
     CommandExecutionContext,
     CommandReturn,
+    GModelRoot,
     ILogger,
     MorphEdgesAnimation,
-    GModelRoot,
     TYPES,
     UpdateAnimationData,
     UpdateModelAction,
     UpdateModelCommand,
     toTypeGuard
 } from '@eclipse-glsp/sprotty';
+import { inject, injectable, optional } from 'inversify';
 import { IFeedbackActionDispatcher } from './feedback-action-dispatcher';
 import { FeedbackCommand } from './feedback-command';
 
@@ -83,7 +83,7 @@ export class FeedbackAwareUpdateModelCommand extends UpdateModelCommand {
                 .map(handler => handler.handle(action));
             result.push(...commands);
         });
-        // sort commands descanting by priority
+        // sort commands descending by priority
         return result.sort((a, b) => this.getPriority(b) - this.getPriority(a));
     }
 

From 81b2264e7b23abc5c568469745f24add832be3b4 Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Sun, 10 Mar 2024 12:18:14 +0100
Subject: [PATCH 2/7] Apply PR feedback

- Avoid code duplication by moving function into feedback dispatcher
---
 .../feedback/feedback-action-dispatcher.ts    | 59 ++++++++++++++++++-
 .../src/base/feedback/feedback-command.ts     |  7 ++-
 .../src/base/feedback/set-model-command.ts    | 48 +++------------
 .../src/base/feedback/update-model-command.ts | 39 ++----------
 packages/client/src/base/priority.ts          | 50 ++++++++++++++++
 .../client/src/base/selection-service.spec.ts |  8 ++-
 6 files changed, 131 insertions(+), 80 deletions(-)
 create mode 100644 packages/client/src/base/priority.ts

diff --git a/packages/client/src/base/feedback/feedback-action-dispatcher.ts b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
index 708b2bb9..8d19e4ea 100644
--- a/packages/client/src/base/feedback/feedback-action-dispatcher.ts
+++ b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
@@ -13,8 +13,22 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
+import {
+    Action,
+    ActionHandlerRegistry,
+    Command,
+    CommandActionHandler,
+    CommandExecutionContext,
+    Disposable,
+    IActionDispatcher,
+    ICommand,
+    ILogger,
+    TYPES,
+    toTypeGuard
+} from '@eclipse-glsp/sprotty';
 import { inject, injectable } from 'inversify';
-import { Action, Disposable, IActionDispatcher, ILogger, TYPES } from '@eclipse-glsp/sprotty';
+import { Prioritized } from '../priority';
+import { FeedbackCommand } from './feedback-command';
 
 export interface IFeedbackEmitter {}
 
@@ -57,6 +71,16 @@ export interface IFeedbackActionDispatcher {
      * Retrieve all `actions` sent out by currently registered `feedbackEmitter`.
      */
     getRegisteredFeedback(): Action[];
+
+    /**
+     * Retrieves all commands based on the registered feedback actions, ordered by their priority (highest priority first).
+     */
+    getFeedbackCommands(): Command[];
+
+    /**
+     * Applies all current feedback commands to the given command execution context.
+     */
+    applyFeedbackCommands(context: CommandExecutionContext): Promise<void>;
 }
 
 @injectable()
@@ -66,6 +90,12 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
     @inject(TYPES.IActionDispatcherProvider) protected actionDispatcher: () => Promise<IActionDispatcher>;
     @inject(TYPES.ILogger) protected logger: ILogger;
 
+    protected actionHandlerRegistry?: ActionHandlerRegistry;
+
+    constructor(@inject(TYPES.ActionHandlerRegistryProvider) actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>) {
+        actionHandlerRegistryProvider().then(registry => (this.actionHandlerRegistry = registry));
+    }
+
     registerFeedback(feedbackEmitter: IFeedbackEmitter, feedbackActions: Action[], cleanupActions?: Action[] | undefined): Disposable {
         if (feedbackActions.length > 0) {
             this.registeredFeedback.set(feedbackEmitter, feedbackActions);
@@ -97,6 +127,33 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
         return result;
     }
 
+    getFeedbackCommands(): Command[] {
+        return this.getRegisteredFeedback()
+            .flatMap(action => this.actionToCommands(action))
+            .sort(Prioritized.sortDesc);
+    }
+
+    async applyFeedbackCommands(context: CommandExecutionContext): Promise<void> {
+        const feedbackCommands = this.getFeedbackCommands() ?? [];
+        if (feedbackCommands?.length > 0) {
+            const results = feedbackCommands.map(command => command.execute(context));
+            await Promise.all(results);
+        }
+    }
+
+    protected actionToCommands(action: Action): ICommand[] {
+        return (
+            this.actionHandlerRegistry
+                ?.get(action.kind)
+                .filter(toTypeGuard(CommandActionHandler))
+                .map(handler => handler.handle(action)) ?? []
+        );
+    }
+
+    protected getPriority(command: Partial<FeedbackCommand>): number {
+        return command.priority ?? 0;
+    }
+
     protected async dispatchFeedback(actions: Action[], feedbackEmitter: IFeedbackEmitter): Promise<void> {
         try {
             const actionDispatcher = await this.actionDispatcher();
diff --git a/packages/client/src/base/feedback/feedback-command.ts b/packages/client/src/base/feedback/feedback-command.ts
index 1cd192a6..104252ee 100644
--- a/packages/client/src/base/feedback/feedback-command.ts
+++ b/packages/client/src/base/feedback/feedback-command.ts
@@ -14,10 +14,11 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 import { Command, CommandExecutionContext, CommandReturn } from '@eclipse-glsp/sprotty';
+import { Prioritized } from '../priority';
 
-export abstract class FeedbackCommand extends Command {
-    // used by the `FeedbackAwareUpdateModelCommand`
-    readonly priority: number = 0;
+export abstract class FeedbackCommand extends Command implements Prioritized {
+    // used by the `FeedbackActionDispatcher`
+    readonly priority: number = Prioritized.DEFAULT_PRIORITY;
 
     undo(context: CommandExecutionContext): CommandReturn {
         return context.root;
diff --git a/packages/client/src/base/feedback/set-model-command.ts b/packages/client/src/base/feedback/set-model-command.ts
index ef680272..71c37d24 100644
--- a/packages/client/src/base/feedback/set-model-command.ts
+++ b/packages/client/src/base/feedback/set-model-command.ts
@@ -13,13 +13,9 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import {
-    ActionHandlerRegistry, Command, CommandActionHandler, CommandExecutionContext, GModelRoot, ILogger, SetModelAction,
-    SetModelCommand, TYPES, toTypeGuard
-} from '@eclipse-glsp/sprotty';
+import { CommandExecutionContext, GModelRoot, ILogger, SetModelAction, SetModelCommand, TYPES } from '@eclipse-glsp/sprotty';
 import { inject, injectable, optional } from 'inversify';
 import { IFeedbackActionDispatcher } from './feedback-action-dispatcher';
-import { FeedbackCommand } from './feedback-command';
 
 @injectable()
 export class FeedbackAwareSetModelCommand extends SetModelCommand {
@@ -28,17 +24,10 @@ export class FeedbackAwareSetModelCommand extends SetModelCommand {
 
     @inject(TYPES.IFeedbackActionDispatcher)
     @optional()
-    protected feedbackActionDispatcher: IFeedbackActionDispatcher;
+    protected feedbackActionDispatcher?: IFeedbackActionDispatcher;
 
-    protected actionHandlerRegistry?: ActionHandlerRegistry;
-
-    constructor(
-        @inject(TYPES.Action) action: SetModelAction,
-        @inject(TYPES.ActionHandlerRegistryProvider)
-        actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>
-    ) {
+    constructor(@inject(TYPES.Action) action: SetModelAction) {
         super(action);
-        actionHandlerRegistryProvider().then(registry => (this.actionHandlerRegistry = registry));
     }
 
     override execute(context: CommandExecutionContext): GModelRoot {
@@ -48,32 +37,9 @@ export class FeedbackAwareSetModelCommand extends SetModelCommand {
     }
 
     protected applyFeedback(newRoot: GModelRoot, context: CommandExecutionContext): void {
-        if (this.feedbackActionDispatcher && this.actionHandlerRegistry) {
-            // Create a temporary context which defines the `newRoot` as `root`
-            // This way we do not corrupt the redo/undo behavior of the super class
-            const tempContext: CommandExecutionContext = {
-                ...context,
-                root: newRoot
-            };
-            const feedbackCommands = this.getFeedbackCommands(this.actionHandlerRegistry);
-            feedbackCommands.forEach(command => command.execute(tempContext));
-        }
-    }
-
-    protected getFeedbackCommands(registry: ActionHandlerRegistry): Command[] {
-        const result: Command[] = [];
-        this.feedbackActionDispatcher.getRegisteredFeedback().forEach(action => {
-            const commands = registry
-                .get(action.kind)
-                .filter<CommandActionHandler>(toTypeGuard(CommandActionHandler))
-                .map(handler => handler.handle(action));
-            result.push(...commands);
-        });
-        // sort commands descending by priority
-        return result.sort((a, b) => this.getPriority(b) - this.getPriority(a));
-    }
-
-    protected getPriority(command: Partial<FeedbackCommand>): number {
-        return command.priority ?? 0;
+        // Create a temporary context which defines the `newRoot` as `root`
+        // This way we do not corrupt the redo/undo behavior of the super class
+        const tempContext: CommandExecutionContext = { ...context, root: newRoot };
+        this.feedbackActionDispatcher?.applyFeedbackCommands(tempContext);
     }
 }
diff --git a/packages/client/src/base/feedback/update-model-command.ts b/packages/client/src/base/feedback/update-model-command.ts
index cebab5e7..6c994215 100644
--- a/packages/client/src/base/feedback/update-model-command.ts
+++ b/packages/client/src/base/feedback/update-model-command.ts
@@ -16,8 +16,6 @@
 import {
     ActionHandlerRegistry,
     Animation,
-    Command,
-    CommandActionHandler,
     CommandExecutionContext,
     CommandReturn,
     GModelRoot,
@@ -26,12 +24,10 @@ import {
     TYPES,
     UpdateAnimationData,
     UpdateModelAction,
-    UpdateModelCommand,
-    toTypeGuard
+    UpdateModelCommand
 } from '@eclipse-glsp/sprotty';
 import { inject, injectable, optional } from 'inversify';
 import { IFeedbackActionDispatcher } from './feedback-action-dispatcher';
-import { FeedbackCommand } from './feedback-command';
 
 /**
  * A special {@link UpdateModelCommand} that retrieves all registered {@link Action}s from the {@link IFeedbackActionDispatcher}
@@ -59,38 +55,13 @@ export class FeedbackAwareUpdateModelCommand extends UpdateModelCommand {
     }
 
     protected override performUpdate(oldRoot: GModelRoot, newRoot: GModelRoot, context: CommandExecutionContext): CommandReturn {
-        if (this.feedbackActionDispatcher && this.actionHandlerRegistry) {
-            // Create a temporary context which defines the `newRoot` as `root`
-            // This way we do not corrupt the redo/undo behavior of the super class
-            const tempContext: CommandExecutionContext = {
-                ...context,
-                root: newRoot
-            };
-
-            const feedbackCommands = this.getFeedbackCommands(this.actionHandlerRegistry);
-            feedbackCommands.forEach(command => command.execute(tempContext));
-        }
-
+        // Create a temporary context which defines the `newRoot` as `root`
+        // This way we do not corrupt the redo/undo behavior of the super class
+        const tempContext: CommandExecutionContext = { ...context, root: newRoot };
+        this.feedbackActionDispatcher?.applyFeedbackCommands(tempContext);
         return super.performUpdate(oldRoot, newRoot, context);
     }
 
-    protected getFeedbackCommands(registry: ActionHandlerRegistry): Command[] {
-        const result: Command[] = [];
-        this.feedbackActionDispatcher.getRegisteredFeedback().forEach(action => {
-            const commands = registry
-                .get(action.kind)
-                .filter<CommandActionHandler>(toTypeGuard(CommandActionHandler))
-                .map(handler => handler.handle(action));
-            result.push(...commands);
-        });
-        // sort commands descending by priority
-        return result.sort((a, b) => this.getPriority(b) - this.getPriority(a));
-    }
-
-    protected getPriority(command: Partial<FeedbackCommand>): number {
-        return command.priority ?? 0;
-    }
-
     // Override the `createAnimations` implementation and remove the animation for edge morphing. Otherwise routing & reconnect
     // handles flicker after each server update.
     protected override createAnimations(data: UpdateAnimationData, root: GModelRoot, context: CommandExecutionContext): Animation[] {
diff --git a/packages/client/src/base/priority.ts b/packages/client/src/base/priority.ts
new file mode 100644
index 00000000..d67d7694
--- /dev/null
+++ b/packages/client/src/base/priority.ts
@@ -0,0 +1,50 @@
+/********************************************************************************
+ * Copyright (c) 2024 EclipseSource and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { AnyObject, hasNumberProp } from '@eclipse-glsp/sprotty';
+
+/**
+ * A common interface for objects that can be orderable by a priority.
+ */
+export interface Prioritized {
+    priority: number;
+}
+
+export namespace Prioritized {
+    export const DEFAULT_PRIORITY = 0;
+    export function is(object: unknown): object is Prioritized {
+        return AnyObject.is(object) && hasNumberProp(object, 'priority');
+    }
+
+    /**
+     * Tries to retrieve the priority form the given object. If the object
+     * implements the {@link Prioritized} interface the corresponding rank is returned
+     * otherwise the {@link DEFAULT_RANK} is returned.
+     * @param object
+     */
+    export function getPriority(object: unknown): number {
+        return is(object) ? object.priority : DEFAULT_PRIORITY;
+    }
+
+    /** Sort function for lowest priority first. */
+    export const sortAsc = (left: unknown, right: unknown): number => getPriority(left) - getPriority(right);
+
+    /** Sort function for highest priority first. */
+    export const sortDesc = (left: unknown, right: unknown): number => getPriority(right) - getPriority(left);
+
+    /** Default sort function for priority: Highest priority first */
+    export const sort = sortDesc;
+}
diff --git a/packages/client/src/base/selection-service.spec.ts b/packages/client/src/base/selection-service.spec.ts
index eb2ee082..d33bb00d 100644
--- a/packages/client/src/base/selection-service.spec.ts
+++ b/packages/client/src/base/selection-service.spec.ts
@@ -13,7 +13,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { Action, Disposable, GModelRoot, GNode, TYPES, initializeContainer } from '@eclipse-glsp/sprotty';
+import { Action, Command, CommandExecutionContext, Disposable, GModelRoot, GNode, TYPES, initializeContainer } from '@eclipse-glsp/sprotty';
 import { AssertionError, expect } from 'chai';
 import { Container, injectable } from 'inversify';
 import * as sinon from 'sinon';
@@ -44,6 +44,12 @@ class MockFeedbackActionDispatcher implements IFeedbackActionDispatcher {
         const actions = this.getRegisteredFeedback();
         return actions.length === 1 ? (actions[0] as SelectFeedbackAction) : undefined;
     }
+
+    getFeedbackCommands(): Command[] {
+        return [];
+    }
+
+    async applyFeedbackCommands(context: CommandExecutionContext): Promise<void> {}
 }
 
 class MockSelectionListener implements ISelectionListener {

From f361c078600e24263c9009f228b47c0d84886857 Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Sun, 10 Mar 2024 21:03:38 +0100
Subject: [PATCH 3/7] Apply further PR feedback: Use ranked for feedback
 commands

---
 CHANGELOG.md                                  |  4 ++
 .../feedback/feedback-action-dispatcher.ts    | 11 ++--
 .../src/base/feedback/feedback-command.ts     | 20 ++++++--
 packages/client/src/base/priority.ts          | 50 -------------------
 packages/client/src/base/ranked.ts            | 14 ++++++
 .../bounds/set-bounds-feedback-command.ts     |  3 +-
 .../src/features/hints/type-hint-provider.ts  | 10 ++--
 7 files changed, 43 insertions(+), 69 deletions(-)
 delete mode 100644 packages/client/src/base/priority.ts

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d6546611..851af523 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,10 @@
 -   [diagram] Restructure some tools to have a more common infrastructure and support helper lines [#306](https://github.com/eclipse-glsp/glsp-client/pull/306)
 -   [diagram] Fix a bug in `SelectionService` that caused issues with inversify when injecting certain services (e.g. `ActionDispatcher`) in `SelectionChangeListener` implementations [#305](https://github.com/eclipse-glsp/glsp-client/pull/305)
 -   [diagram] Ensure that the `SelectionService` does not trigger a change event if the selection did not change on model update [#313](https://github.com/eclipse-glsp/glsp-client/pull/313)
+-   [API] Apply feedback commands already on `SetModelCommand` and unify `rank` and `priority` property [#323](https://github.com/eclipse-glsp/glsp-client/pull/322).
+    -   Method `FeedbackAwareUpdateModelCommand.getFeedbackCommands` was move to `IFeedbackEmitter` for re-use, resulting in two new methods: `getFeedbackCommands` and `applyFeedbackCommands`.
+    -   Method `FeedbackAwareUpdateModelCommand.getPriority` was replaced by a generic `rank` property and the `Ranked` namespace.
+    -   The `priority` property (higher priority equals earlier execution) in `FeedbackCommand` was superseeded by a `rank` property (lower rank equals earlier execution).
 
 ## [v2.0.0 - 14/10/2023](https://github.com/eclipse-glsp/glsp-client/releases/tag/v2.0.0)
 
diff --git a/packages/client/src/base/feedback/feedback-action-dispatcher.ts b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
index 8d19e4ea..3b017c98 100644
--- a/packages/client/src/base/feedback/feedback-action-dispatcher.ts
+++ b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
@@ -27,8 +27,7 @@ import {
     toTypeGuard
 } from '@eclipse-glsp/sprotty';
 import { inject, injectable } from 'inversify';
-import { Prioritized } from '../priority';
-import { FeedbackCommand } from './feedback-command';
+import { getFeedbackRank } from './feedback-command';
 
 export interface IFeedbackEmitter {}
 
@@ -73,7 +72,7 @@ export interface IFeedbackActionDispatcher {
     getRegisteredFeedback(): Action[];
 
     /**
-     * Retrieves all commands based on the registered feedback actions, ordered by their priority (highest priority first).
+     * Retrieves all commands based on the registered feedback actions, ordered by their rank (lowest rank first).
      */
     getFeedbackCommands(): Command[];
 
@@ -130,7 +129,7 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
     getFeedbackCommands(): Command[] {
         return this.getRegisteredFeedback()
             .flatMap(action => this.actionToCommands(action))
-            .sort(Prioritized.sortDesc);
+            .sort((left, right) => getFeedbackRank(left) - getFeedbackRank(right));
     }
 
     async applyFeedbackCommands(context: CommandExecutionContext): Promise<void> {
@@ -150,10 +149,6 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
         );
     }
 
-    protected getPriority(command: Partial<FeedbackCommand>): number {
-        return command.priority ?? 0;
-    }
-
     protected async dispatchFeedback(actions: Action[], feedbackEmitter: IFeedbackEmitter): Promise<void> {
         try {
             const actionDispatcher = await this.actionDispatcher();
diff --git a/packages/client/src/base/feedback/feedback-command.ts b/packages/client/src/base/feedback/feedback-command.ts
index 104252ee..2d033982 100644
--- a/packages/client/src/base/feedback/feedback-command.ts
+++ b/packages/client/src/base/feedback/feedback-command.ts
@@ -13,12 +13,16 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { Command, CommandExecutionContext, CommandReturn } from '@eclipse-glsp/sprotty';
-import { Prioritized } from '../priority';
+/* eslint-disable deprecation/deprecation */
+import { Command, CommandExecutionContext, CommandReturn, ICommand } from '@eclipse-glsp/sprotty';
+import { Ranked } from '../ranked';
 
-export abstract class FeedbackCommand extends Command implements Prioritized {
-    // used by the `FeedbackActionDispatcher`
-    readonly priority: number = Prioritized.DEFAULT_PRIORITY;
+export abstract class FeedbackCommand extends Command implements Ranked {
+    /** @deprecated Use rank instead. Please note that a lower rank implies higher priority, so the order is reversed. */
+    readonly priority?: number = 0;
+
+    // backwards compatibility: convert any existing priority to an equivalent rank
+    readonly rank: number = this.priority ? -this.priority : Ranked.DEFAULT_RANK;
 
     undo(context: CommandExecutionContext): CommandReturn {
         return context.root;
@@ -28,3 +32,9 @@ export abstract class FeedbackCommand extends Command implements Prioritized {
         return context.root;
     }
 }
+
+/** Used for backwards compatibility, otherwise use Ranked.getRank or Ranked sort functions. */
+export function getFeedbackRank(command: ICommand): number {
+    const feedbackCommand = command as Partial<FeedbackCommand>;
+    return feedbackCommand?.priority ? -feedbackCommand.priority : feedbackCommand.rank ?? Ranked.DEFAULT_RANK;
+}
diff --git a/packages/client/src/base/priority.ts b/packages/client/src/base/priority.ts
deleted file mode 100644
index d67d7694..00000000
--- a/packages/client/src/base/priority.ts
+++ /dev/null
@@ -1,50 +0,0 @@
-/********************************************************************************
- * Copyright (c) 2024 EclipseSource and others.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v. 2.0 which is available at
- * http://www.eclipse.org/legal/epl-2.0.
- *
- * This Source Code may also be made available under the following Secondary
- * Licenses when the conditions for such availability set forth in the Eclipse
- * Public License v. 2.0 are satisfied: GNU General Public License, version 2
- * with the GNU Classpath Exception which is available at
- * https://www.gnu.org/software/classpath/license.html.
- *
- * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
- ********************************************************************************/
-
-import { AnyObject, hasNumberProp } from '@eclipse-glsp/sprotty';
-
-/**
- * A common interface for objects that can be orderable by a priority.
- */
-export interface Prioritized {
-    priority: number;
-}
-
-export namespace Prioritized {
-    export const DEFAULT_PRIORITY = 0;
-    export function is(object: unknown): object is Prioritized {
-        return AnyObject.is(object) && hasNumberProp(object, 'priority');
-    }
-
-    /**
-     * Tries to retrieve the priority form the given object. If the object
-     * implements the {@link Prioritized} interface the corresponding rank is returned
-     * otherwise the {@link DEFAULT_RANK} is returned.
-     * @param object
-     */
-    export function getPriority(object: unknown): number {
-        return is(object) ? object.priority : DEFAULT_PRIORITY;
-    }
-
-    /** Sort function for lowest priority first. */
-    export const sortAsc = (left: unknown, right: unknown): number => getPriority(left) - getPriority(right);
-
-    /** Sort function for highest priority first. */
-    export const sortDesc = (left: unknown, right: unknown): number => getPriority(right) - getPriority(left);
-
-    /** Default sort function for priority: Highest priority first */
-    export const sort = sortDesc;
-}
diff --git a/packages/client/src/base/ranked.ts b/packages/client/src/base/ranked.ts
index f1914fbb..759236b8 100644
--- a/packages/client/src/base/ranked.ts
+++ b/packages/client/src/base/ranked.ts
@@ -21,11 +21,16 @@ import { AnyObject, hasNumberProp } from '@eclipse-glsp/sprotty';
  * orderable by a type or rank/priority.
  */
 export interface Ranked {
+    /**
+     * A rank implies the position of this element within a sequence of other ranked elements.
+     * A lower rank implies a position earlier in the list.
+     */
     rank: number;
 }
 
 export namespace Ranked {
     export const DEFAULT_RANK = 0;
+
     export function is(object: unknown): object is Ranked {
         return AnyObject.is(object) && hasNumberProp(object, 'rank');
     }
@@ -39,4 +44,13 @@ export namespace Ranked {
     export function getRank(object: unknown): number {
         return is(object) ? object.rank : DEFAULT_RANK;
     }
+
+    /** Sort function for lowest rank first. */
+    export const sortAsc = (left: unknown, right: unknown): number => getRank(left) - getRank(right);
+
+    /** Sort function for highest rank first. */
+    export const sortDesc = (left: unknown, right: unknown): number => getRank(right) - getRank(left);
+
+    /** Default sort function for rank: Lowest rank first */
+    export const sort = sortAsc;
 }
diff --git a/packages/client/src/features/bounds/set-bounds-feedback-command.ts b/packages/client/src/features/bounds/set-bounds-feedback-command.ts
index 62b20e4b..de642470 100644
--- a/packages/client/src/features/bounds/set-bounds-feedback-command.ts
+++ b/packages/client/src/features/bounds/set-bounds-feedback-command.ts
@@ -24,6 +24,7 @@ import {
     isLayoutContainer
 } from '@eclipse-glsp/sprotty';
 import { inject, injectable } from 'inversify';
+import { Ranked } from '../../base';
 import { GLSPActionDispatcher } from '../../base/action-dispatcher';
 import { FeedbackCommand } from '../../base/feedback/feedback-command';
 import { LocalRequestBoundsAction } from './local-bounds';
@@ -48,7 +49,7 @@ export namespace SetBoundsFeedbackAction {
 export class SetBoundsFeedbackCommand extends SetBoundsCommand implements FeedbackCommand {
     static override readonly KIND: string = SetBoundsFeedbackAction.KIND;
 
-    readonly priority: number = 0;
+    readonly rank: number = Ranked.DEFAULT_RANK;
 
     @inject(TYPES.IActionDispatcher) protected actionDispatcher: GLSPActionDispatcher;
 
diff --git a/packages/client/src/features/hints/type-hint-provider.ts b/packages/client/src/features/hints/type-hint-provider.ts
index 093298d8..772ffc36 100644
--- a/packages/client/src/features/hints/type-hint-provider.ts
+++ b/packages/client/src/features/hints/type-hint-provider.ts
@@ -13,19 +13,18 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { inject, injectable } from 'inversify';
 import {
     Action,
     CommandExecutionContext,
     Connectable,
     EdgeTypeHint,
-    IActionHandler,
-    RequestTypeHintsAction,
     GModelElement,
     GModelElementSchema,
     GModelRoot,
     GRoutableElement,
     GShapeElement,
+    IActionHandler,
+    RequestTypeHintsAction,
     SetTypeHintsAction,
     ShapeTypeHint,
     TYPES,
@@ -36,16 +35,17 @@ import {
     isConnectable,
     moveFeature
 } from '@eclipse-glsp/sprotty';
+import { inject, injectable } from 'inversify';
 
 import { GLSPActionDispatcher } from '../../base/action-dispatcher';
 import { IFeedbackActionDispatcher } from '../../base/feedback/feedback-action-dispatcher';
 import { FeedbackCommand } from '../../base/feedback/feedback-command';
 import { IDiagramStartup } from '../../base/model/diagram-loader';
+import { GEdge } from '../../model';
 import { getElementTypeId } from '../../utils/gmodel-util';
 import { resizeFeature } from '../change-bounds/model';
 import { reconnectFeature } from '../reconnect/model';
 import { containerFeature, isContainable, reparentFeature } from './model';
-import { GEdge } from '../../model';
 
 /**
  * Is dispatched by the {@link TypeHintProvider} to apply the type hints received from the server
@@ -78,7 +78,7 @@ type CanConnectFn = Connectable['canConnect'];
 @injectable()
 export class ApplyTypeHintsCommand extends FeedbackCommand {
     public static KIND = ApplyTypeHintsAction.KIND;
-    public override readonly priority = 10;
+    public override readonly rank: number = -10;
 
     @inject(TYPES.ITypeHintProvider)
     protected typeHintProvider: ITypeHintProvider;

From 8fa1e67d45a38b038934bd0e2bc528006cc75ae7 Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Mon, 11 Mar 2024 12:55:30 +0100
Subject: [PATCH 4/7] Further fixing: Avoid circular dependency with
 contribution provider

---
 .../src/base/action-handler-registry.ts       | 43 +++++++++-
 packages/client/src/base/default.module.ts    |  4 +
 .../feedback/feedback-action-dispatcher.ts    |  6 +-
 .../src/base/feedback/update-model-command.ts | 10 +--
 .../src/utils/contribution-provider.ts        | 79 +++++++++++++++++++
 5 files changed, 126 insertions(+), 16 deletions(-)
 create mode 100644 packages/protocol/src/utils/contribution-provider.ts

diff --git a/packages/client/src/base/action-handler-registry.ts b/packages/client/src/base/action-handler-registry.ts
index 7b130324..7449cba2 100644
--- a/packages/client/src/base/action-handler-registry.ts
+++ b/packages/client/src/base/action-handler-registry.ts
@@ -14,17 +14,56 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { injectable } from 'inversify';
-import { ActionHandlerRegistry } from '@eclipse-glsp/sprotty';
+import { ContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
+import { ActionHandlerRegistration, ActionHandlerRegistry, IActionHandler, IActionHandlerInitializer, TYPES } from '@eclipse-glsp/sprotty';
+import { inject, injectable, named } from 'inversify';
 
 @injectable()
 export class GLSPActionHandlerRegistry extends ActionHandlerRegistry {
+    @inject(ContributionProvider)
+    @named(TYPES.ActionHandlerRegistration)
+    protected readonly registrations: ContributionProvider<ActionHandlerRegistration>;
+
+    @inject(ContributionProvider)
+    @named(TYPES.IActionHandlerInitializer)
+    protected readonly initializers: ContributionProvider<IActionHandlerInitializer>;
+
+    protected initialized = false;
+
+    constructor() {
+        super([], []);
+    }
+
+    protected init(): void {
+        if (!this.initialized) {
+            this.initialized = true;
+            this.registrations.getContributions().forEach(registration => this.register(registration.actionKind, registration.factory()));
+            this.initializers.getContributions().forEach(initializer => this.initializeActionHandler(initializer));
+        }
+    }
+
+    override register(key: string, instance: IActionHandler): void {
+        this.init();
+        super.register(key, instance);
+    }
+
+    override get(key: string): IActionHandler[] {
+        this.init();
+        return super.get(key);
+    }
+
+    override initializeActionHandler(initializer: IActionHandlerInitializer): void {
+        this.init();
+        super.initializeActionHandler(initializer);
+    }
+
     /**
      * Retrieve a set of all action kinds for which (at least) one
      * handler is registered
      * @returns the set of handled action kinds
      */
     getHandledActionKinds(): string[] {
+        this.init();
         return Array.from(this.elements.keys());
     }
 }
diff --git a/packages/client/src/base/default.module.ts b/packages/client/src/base/default.module.ts
index 0fa2178f..1c5c9955 100644
--- a/packages/client/src/base/default.module.ts
+++ b/packages/client/src/base/default.module.ts
@@ -13,6 +13,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
+import { bindContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
 import {
     ActionHandlerRegistry,
     FeatureModule,
@@ -87,6 +88,9 @@ export const defaultModule = new FeatureModule((bind, unbind, isBound, rebind, .
     bindOrRebind(context, TYPES.ICommandStack).to(GLSPCommandStack).inSingletonScope();
     bind(GLSPActionDispatcher).toSelf().inSingletonScope();
     bindOrRebind(context, TYPES.IActionDispatcher).toService(GLSPActionDispatcher);
+
+    bindContributionProvider(bind, TYPES.ActionHandlerRegistration);
+    bindContributionProvider(bind, TYPES.IActionHandlerInitializer);
     bind(GLSPActionHandlerRegistry).toSelf().inSingletonScope();
     bindOrRebind(context, ActionHandlerRegistry).toService(GLSPActionHandlerRegistry);
 
diff --git a/packages/client/src/base/feedback/feedback-action-dispatcher.ts b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
index 3b017c98..1dcd22d9 100644
--- a/packages/client/src/base/feedback/feedback-action-dispatcher.ts
+++ b/packages/client/src/base/feedback/feedback-action-dispatcher.ts
@@ -89,11 +89,7 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher {
     @inject(TYPES.IActionDispatcherProvider) protected actionDispatcher: () => Promise<IActionDispatcher>;
     @inject(TYPES.ILogger) protected logger: ILogger;
 
-    protected actionHandlerRegistry?: ActionHandlerRegistry;
-
-    constructor(@inject(TYPES.ActionHandlerRegistryProvider) actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>) {
-        actionHandlerRegistryProvider().then(registry => (this.actionHandlerRegistry = registry));
-    }
+    @inject(ActionHandlerRegistry) protected actionHandlerRegistry: ActionHandlerRegistry;
 
     registerFeedback(feedbackEmitter: IFeedbackEmitter, feedbackActions: Action[], cleanupActions?: Action[] | undefined): Disposable {
         if (feedbackActions.length > 0) {
diff --git a/packages/client/src/base/feedback/update-model-command.ts b/packages/client/src/base/feedback/update-model-command.ts
index 6c994215..d7666786 100644
--- a/packages/client/src/base/feedback/update-model-command.ts
+++ b/packages/client/src/base/feedback/update-model-command.ts
@@ -14,7 +14,6 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 import {
-    ActionHandlerRegistry,
     Animation,
     CommandExecutionContext,
     CommandReturn,
@@ -43,15 +42,8 @@ export class FeedbackAwareUpdateModelCommand extends UpdateModelCommand {
     @optional()
     protected feedbackActionDispatcher: IFeedbackActionDispatcher;
 
-    protected actionHandlerRegistry?: ActionHandlerRegistry;
-
-    constructor(
-        @inject(TYPES.Action) action: UpdateModelAction,
-        @inject(TYPES.ActionHandlerRegistryProvider)
-        actionHandlerRegistryProvider: () => Promise<ActionHandlerRegistry>
-    ) {
+    constructor(@inject(TYPES.Action) action: UpdateModelAction) {
         super({ animate: true, ...action });
-        actionHandlerRegistryProvider().then(registry => (this.actionHandlerRegistry = registry));
     }
 
     protected override performUpdate(oldRoot: GModelRoot, newRoot: GModelRoot, context: CommandExecutionContext): CommandReturn {
diff --git a/packages/protocol/src/utils/contribution-provider.ts b/packages/protocol/src/utils/contribution-provider.ts
new file mode 100644
index 00000000..f58404e0
--- /dev/null
+++ b/packages/protocol/src/utils/contribution-provider.ts
@@ -0,0 +1,79 @@
+/*******************************************************************************
+ * Copyright (C) 2017 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
+ *******************************************************************************/
+// eslint-disable-next-line max-len
+// from https://github.com/eclipse-theia/theia/blob/9ff0cedff1d591b0eb4be97a05f6d992789d0a24/packages/core/src/common/contribution-provider.ts
+
+import { interfaces } from 'inversify';
+
+export const ContributionProvider = Symbol('ContributionProvider');
+
+export interface ContributionProvider<T extends object> {
+    /**
+     * @param recursive `true` if the contributions should be collected from the parent containers as well. Otherwise, `false`.
+     * It is `false` by default.
+     */
+    getContributions(recursive?: boolean): T[];
+}
+
+class ContainerBasedContributionProvider<T extends object> implements ContributionProvider<T> {
+    protected services: T[] | undefined;
+
+    constructor(
+        protected readonly serviceIdentifier: interfaces.ServiceIdentifier<T>,
+        protected readonly container: interfaces.Container
+    ) {}
+
+    getContributions(recursive?: boolean): T[] {
+        if (this.services === undefined) {
+            const currentServices: T[] = [];
+            let currentContainer: interfaces.Container | null = this.container;
+            // eslint-disable-next-line no-null/no-null
+            while (currentContainer !== null) {
+                if (currentContainer.isBound(this.serviceIdentifier)) {
+                    try {
+                        currentServices.push(...currentContainer.getAll(this.serviceIdentifier));
+                    } catch (error) {
+                        console.error(error);
+                    }
+                }
+                // eslint-disable-next-line no-null/no-null
+                currentContainer = recursive === true ? currentContainer.parent : null;
+            }
+            this.services = currentServices;
+        }
+        return this.services;
+    }
+}
+
+export type Bindable = interfaces.Bind | interfaces.Container;
+export namespace Bindable {
+    export function isContainer(arg: Bindable): arg is interfaces.Container {
+        return (
+            typeof arg !== 'function' &&
+            // https://github.com/eclipse-theia/theia/issues/3204#issue-371029654
+            // In InversifyJS `4.14.0` containers no longer have a property `guid`.
+            ('guid' in arg || 'parent' in arg)
+        );
+    }
+}
+
+export function bindContributionProvider(bindable: Bindable, id: symbol): void {
+    const bindingToSyntax = Bindable.isContainer(bindable) ? bindable.bind(ContributionProvider) : bindable(ContributionProvider);
+    bindingToSyntax
+        .toDynamicValue(ctx => new ContainerBasedContributionProvider(id, ctx.container))
+        .inSingletonScope()
+        .whenTargetNamed(id);
+}

From b6c7b189aba255321ec20e17805c92583a36d4aa Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Mon, 11 Mar 2024 14:57:08 +0100
Subject: [PATCH 5/7] Apply further PR feedback

- Avoid unnecessary types in contribution-provider, re-use GLSP ones
- Use undefined instead of 'null'
- Export 'ContributionProvider' also as 'TYPES.IContributionProvider'
- Adapt copyright header
---
 .../src/base/action-handler-registry.ts       | 18 +++++++----
 packages/glsp-sprotty/src/types.ts            |  8 ++++-
 .../src/utils/contribution-provider.ts        | 30 ++++++-------------
 packages/protocol/src/utils/index.ts          |  1 +
 4 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/packages/client/src/base/action-handler-registry.ts b/packages/client/src/base/action-handler-registry.ts
index 7449cba2..90a2b3c2 100644
--- a/packages/client/src/base/action-handler-registry.ts
+++ b/packages/client/src/base/action-handler-registry.ts
@@ -14,19 +14,25 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import { ContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
-import { ActionHandlerRegistration, ActionHandlerRegistry, IActionHandler, IActionHandlerInitializer, TYPES } from '@eclipse-glsp/sprotty';
+import {
+    ActionHandlerRegistration,
+    ActionHandlerRegistry,
+    IActionHandler,
+    IActionHandlerInitializer,
+    IContributionProvider,
+    TYPES
+} from '@eclipse-glsp/sprotty';
 import { inject, injectable, named } from 'inversify';
 
 @injectable()
 export class GLSPActionHandlerRegistry extends ActionHandlerRegistry {
-    @inject(ContributionProvider)
+    @inject(TYPES.IContributionProvider)
     @named(TYPES.ActionHandlerRegistration)
-    protected readonly registrations: ContributionProvider<ActionHandlerRegistration>;
+    protected readonly registrations: IContributionProvider<ActionHandlerRegistration>;
 
-    @inject(ContributionProvider)
+    @inject(TYPES.IContributionProvider)
     @named(TYPES.IActionHandlerInitializer)
-    protected readonly initializers: ContributionProvider<IActionHandlerInitializer>;
+    protected readonly initializers: IContributionProvider<IActionHandlerInitializer>;
 
     protected initialized = false;
 
diff --git a/packages/glsp-sprotty/src/types.ts b/packages/glsp-sprotty/src/types.ts
index 8c763eb2..a848f321 100644
--- a/packages/glsp-sprotty/src/types.ts
+++ b/packages/glsp-sprotty/src/types.ts
@@ -13,7 +13,10 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
+import type { ContributionProvider as IContributionProvider } from '@eclipse-glsp/protocol';
+import { ContributionProvider } from '@eclipse-glsp/protocol';
 import { TYPES as SprottyTYPES } from 'sprotty';
+
 /**
  * Reexport of the TYPES namespace of sprotty augments with additional GLSP specific service
  * identifiers.
@@ -46,5 +49,8 @@ export const TYPES = {
     ILocalElementNavigator: Symbol('ILocalElementNavigator'),
     IDiagramOptions: Symbol('IDiagramOptions'),
     IDiagramStartup: Symbol('IDiagramStartup'),
-    IToolManager: Symbol('IToolManager')
+    IToolManager: Symbol('IToolManager'),
+    IContributionProvider: ContributionProvider
 };
+
+export { IContributionProvider };
diff --git a/packages/protocol/src/utils/contribution-provider.ts b/packages/protocol/src/utils/contribution-provider.ts
index f58404e0..e10d1761 100644
--- a/packages/protocol/src/utils/contribution-provider.ts
+++ b/packages/protocol/src/utils/contribution-provider.ts
@@ -1,5 +1,6 @@
 /*******************************************************************************
  * Copyright (C) 2017 TypeFox and others.
+ * Modifications: (c) 2024 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -14,9 +15,10 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
  *******************************************************************************/
 // eslint-disable-next-line max-len
-// from https://github.com/eclipse-theia/theia/blob/9ff0cedff1d591b0eb4be97a05f6d992789d0a24/packages/core/src/common/contribution-provider.ts
+// based on https://github.com/eclipse-theia/theia/blob/9ff0cedff1d591b0eb4be97a05f6d992789d0a24/packages/core/src/common/contribution-provider.ts
 
 import { interfaces } from 'inversify';
+import { BindingContext } from './di-util';
 
 export const ContributionProvider = Symbol('ContributionProvider');
 
@@ -39,9 +41,8 @@ class ContainerBasedContributionProvider<T extends object> implements Contributi
     getContributions(recursive?: boolean): T[] {
         if (this.services === undefined) {
             const currentServices: T[] = [];
-            let currentContainer: interfaces.Container | null = this.container;
-            // eslint-disable-next-line no-null/no-null
-            while (currentContainer !== null) {
+            let currentContainer: interfaces.Container | undefined = this.container;
+            while (currentContainer !== undefined) {
                 if (currentContainer.isBound(this.serviceIdentifier)) {
                     try {
                         currentServices.push(...currentContainer.getAll(this.serviceIdentifier));
@@ -49,8 +50,7 @@ class ContainerBasedContributionProvider<T extends object> implements Contributi
                         console.error(error);
                     }
                 }
-                // eslint-disable-next-line no-null/no-null
-                currentContainer = recursive === true ? currentContainer.parent : null;
+                currentContainer = recursive === true && currentContainer.parent ? currentContainer.parent : undefined;
             }
             this.services = currentServices;
         }
@@ -58,21 +58,9 @@ class ContainerBasedContributionProvider<T extends object> implements Contributi
     }
 }
 
-export type Bindable = interfaces.Bind | interfaces.Container;
-export namespace Bindable {
-    export function isContainer(arg: Bindable): arg is interfaces.Container {
-        return (
-            typeof arg !== 'function' &&
-            // https://github.com/eclipse-theia/theia/issues/3204#issue-371029654
-            // In InversifyJS `4.14.0` containers no longer have a property `guid`.
-            ('guid' in arg || 'parent' in arg)
-        );
-    }
-}
-
-export function bindContributionProvider(bindable: Bindable, id: symbol): void {
-    const bindingToSyntax = Bindable.isContainer(bindable) ? bindable.bind(ContributionProvider) : bindable(ContributionProvider);
-    bindingToSyntax
+export function bindContributionProvider(context: Pick<BindingContext, 'bind'> | interfaces.Bind, id: symbol): void {
+    const bind = typeof context === 'object' ? context.bind.bind(context) : context;
+    bind(ContributionProvider)
         .toDynamicValue(ctx => new ContainerBasedContributionProvider(id, ctx.container))
         .inSingletonScope()
         .whenTargetNamed(id);
diff --git a/packages/protocol/src/utils/index.ts b/packages/protocol/src/utils/index.ts
index 9ba54bb5..3577f00c 100644
--- a/packages/protocol/src/utils/index.ts
+++ b/packages/protocol/src/utils/index.ts
@@ -14,6 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 export * from './array-util';
+export * from './contribution-provider';
 export * from './di-util';
 export * from './disposable';
 export * from './event';

From 4a087589a368acba531dba3a75b6860182ae3b24 Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Mon, 11 Mar 2024 16:29:08 +0100
Subject: [PATCH 6/7] Do not expose 'ContributionProvider' directly

---
 packages/glsp-sprotty/src/types.ts   | 4 ++--
 packages/protocol/src/utils/index.ts | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/glsp-sprotty/src/types.ts b/packages/glsp-sprotty/src/types.ts
index a848f321..c10422de 100644
--- a/packages/glsp-sprotty/src/types.ts
+++ b/packages/glsp-sprotty/src/types.ts
@@ -13,8 +13,8 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import type { ContributionProvider as IContributionProvider } from '@eclipse-glsp/protocol';
-import { ContributionProvider } from '@eclipse-glsp/protocol';
+import type { ContributionProvider as IContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
+import { ContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
 import { TYPES as SprottyTYPES } from 'sprotty';
 
 /**
diff --git a/packages/protocol/src/utils/index.ts b/packages/protocol/src/utils/index.ts
index 3577f00c..a1edfd5a 100644
--- a/packages/protocol/src/utils/index.ts
+++ b/packages/protocol/src/utils/index.ts
@@ -14,7 +14,7 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 export * from './array-util';
-export * from './contribution-provider';
+export { bindContributionProvider } from './contribution-provider';
 export * from './di-util';
 export * from './disposable';
 export * from './event';

From 1c5f299fdf944351332e00721e3010df9522df3b Mon Sep 17 00:00:00 2001
From: Martin Fleck <mfleck@eclipsesource.com>
Date: Tue, 12 Mar 2024 08:28:45 +0100
Subject: [PATCH 7/7] Move 'ContributionProvider' into client package

---
 packages/client/src/base/action-handler-registry.ts    | 10 ++--------
 packages/client/src/base/default.module.ts             |  2 +-
 .../src/utils/contribution-provider.ts                 | 10 ++++------
 packages/glsp-sprotty/src/types.ts                     |  6 +-----
 packages/protocol/src/utils/index.ts                   |  1 -
 5 files changed, 8 insertions(+), 21 deletions(-)
 rename packages/{protocol => client}/src/utils/contribution-provider.ts (92%)

diff --git a/packages/client/src/base/action-handler-registry.ts b/packages/client/src/base/action-handler-registry.ts
index 90a2b3c2..b063fb89 100644
--- a/packages/client/src/base/action-handler-registry.ts
+++ b/packages/client/src/base/action-handler-registry.ts
@@ -14,15 +14,9 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-import {
-    ActionHandlerRegistration,
-    ActionHandlerRegistry,
-    IActionHandler,
-    IActionHandlerInitializer,
-    IContributionProvider,
-    TYPES
-} from '@eclipse-glsp/sprotty';
+import { ActionHandlerRegistration, ActionHandlerRegistry, IActionHandler, IActionHandlerInitializer, TYPES } from '@eclipse-glsp/sprotty';
 import { inject, injectable, named } from 'inversify';
+import { IContributionProvider } from '../utils/contribution-provider';
 
 @injectable()
 export class GLSPActionHandlerRegistry extends ActionHandlerRegistry {
diff --git a/packages/client/src/base/default.module.ts b/packages/client/src/base/default.module.ts
index 1c5c9955..6317f5c0 100644
--- a/packages/client/src/base/default.module.ts
+++ b/packages/client/src/base/default.module.ts
@@ -13,7 +13,6 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { bindContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
 import {
     ActionHandlerRegistry,
     FeatureModule,
@@ -33,6 +32,7 @@ import {
 } from '@eclipse-glsp/sprotty';
 import '@vscode/codicons/dist/codicon.css';
 import '../../css/glsp-sprotty.css';
+import { bindContributionProvider } from '../utils/contribution-provider';
 import { GLSPActionDispatcher } from './action-dispatcher';
 import { GLSPActionHandlerRegistry } from './action-handler-registry';
 import { GLSPCommandStack } from './command-stack';
diff --git a/packages/protocol/src/utils/contribution-provider.ts b/packages/client/src/utils/contribution-provider.ts
similarity index 92%
rename from packages/protocol/src/utils/contribution-provider.ts
rename to packages/client/src/utils/contribution-provider.ts
index e10d1761..a489727a 100644
--- a/packages/protocol/src/utils/contribution-provider.ts
+++ b/packages/client/src/utils/contribution-provider.ts
@@ -17,12 +17,10 @@
 // eslint-disable-next-line max-len
 // based on https://github.com/eclipse-theia/theia/blob/9ff0cedff1d591b0eb4be97a05f6d992789d0a24/packages/core/src/common/contribution-provider.ts
 
+import { BindingContext, TYPES } from '@eclipse-glsp/sprotty';
 import { interfaces } from 'inversify';
-import { BindingContext } from './di-util';
 
-export const ContributionProvider = Symbol('ContributionProvider');
-
-export interface ContributionProvider<T extends object> {
+export interface IContributionProvider<T extends object> {
     /**
      * @param recursive `true` if the contributions should be collected from the parent containers as well. Otherwise, `false`.
      * It is `false` by default.
@@ -30,7 +28,7 @@ export interface ContributionProvider<T extends object> {
     getContributions(recursive?: boolean): T[];
 }
 
-class ContainerBasedContributionProvider<T extends object> implements ContributionProvider<T> {
+class ContainerBasedContributionProvider<T extends object> implements IContributionProvider<T> {
     protected services: T[] | undefined;
 
     constructor(
@@ -60,7 +58,7 @@ class ContainerBasedContributionProvider<T extends object> implements Contributi
 
 export function bindContributionProvider(context: Pick<BindingContext, 'bind'> | interfaces.Bind, id: symbol): void {
     const bind = typeof context === 'object' ? context.bind.bind(context) : context;
-    bind(ContributionProvider)
+    bind(TYPES.IContributionProvider)
         .toDynamicValue(ctx => new ContainerBasedContributionProvider(id, ctx.container))
         .inSingletonScope()
         .whenTargetNamed(id);
diff --git a/packages/glsp-sprotty/src/types.ts b/packages/glsp-sprotty/src/types.ts
index c10422de..2bb7cbec 100644
--- a/packages/glsp-sprotty/src/types.ts
+++ b/packages/glsp-sprotty/src/types.ts
@@ -13,8 +13,6 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import type { ContributionProvider as IContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
-import { ContributionProvider } from '@eclipse-glsp/protocol/lib/utils/contribution-provider';
 import { TYPES as SprottyTYPES } from 'sprotty';
 
 /**
@@ -50,7 +48,5 @@ export const TYPES = {
     IDiagramOptions: Symbol('IDiagramOptions'),
     IDiagramStartup: Symbol('IDiagramStartup'),
     IToolManager: Symbol('IToolManager'),
-    IContributionProvider: ContributionProvider
+    IContributionProvider: Symbol('IContributionProvider')
 };
-
-export { IContributionProvider };
diff --git a/packages/protocol/src/utils/index.ts b/packages/protocol/src/utils/index.ts
index a1edfd5a..9ba54bb5 100644
--- a/packages/protocol/src/utils/index.ts
+++ b/packages/protocol/src/utils/index.ts
@@ -14,7 +14,6 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 export * from './array-util';
-export { bindContributionProvider } from './contribution-provider';
 export * from './di-util';
 export * from './disposable';
 export * from './event';