Apply further PR feedback: Use ranked for feedback commands

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 of sorted commands.
+    -   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)
 

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();

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;
+}

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;
 }

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;
 

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;