fix: library search boosting

Closes #1106

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Author: Akos Kitta

arduino-ide-extension/package.json

@@ -17,7 +17,7 @@
     "build": "tsc && ncp ./src/node/cli-protocol/ ./lib/node/cli-protocol/ && yarn lint",
     "watch": "tsc -w",
     "test": "mocha \"./lib/test/**/*.test.js\"",
-    "test:slow": "mocha \"./lib/test/**/*.slow-test.js\"",
+    "test:slow": "mocha \"./lib/test/**/*.slow-test.js\" --slow 5000",
     "test:watch": "mocha --watch --watch-files lib \"./lib/test/**/*.test.js\""
   },
   "dependencies": {

arduino-ide-extension/src/browser/boards/boards-list-widget.ts

@@ -30,7 +30,6 @@ export class BoardsListWidget extends ListWidget<BoardsPackage, BoardSearch> {
       searchable: service,
       installable: service,
       itemLabel: (item: BoardsPackage) => item.name,
-      itemDeprecated: (item: BoardsPackage) => item.deprecated,
       itemRenderer,
       filterRenderer,
       defaultSearchOptions: { query: '', type: 'All' },

arduino-ide-extension/src/browser/library/library-list-widget.ts

@@ -41,7 +41,6 @@ export class LibraryListWidget extends ListWidget<
       searchable: service,
       installable: service,
       itemLabel: (item: LibraryPackage) => item.name,
-      itemDeprecated: (item: LibraryPackage) => item.deprecated,
       itemRenderer,
       filterRenderer,
       defaultSearchOptions: { query: '', type: 'All', topic: 'All' },

arduino-ide-extension/src/browser/widgets/component-list/component-list.tsx

@@ -147,7 +147,6 @@ export namespace ComponentList {
   export interface Props<T extends ArduinoComponent> {
     readonly items: T[];
     readonly itemLabel: (item: T) => string;
-    readonly itemDeprecated: (item: T) => boolean;
     readonly itemRenderer: ListItemRenderer<T>;
     readonly install: (item: T, version?: Installable.Version) => Promise<void>;
     readonly uninstall: (item: T) => Promise<void>;

arduino-ide-extension/src/browser/widgets/component-list/filterable-list-container.tsx

@@ -82,12 +82,11 @@ export class FilterableListContainer<
   }
 
   protected renderComponentList(): React.ReactNode {
-    const { itemLabel, itemDeprecated, itemRenderer } = this.props;
+    const { itemLabel, itemRenderer } = this.props;
     return (
       <ComponentList<T>
         items={this.state.items}
         itemLabel={itemLabel}
-        itemDeprecated={itemDeprecated}
         itemRenderer={itemRenderer}
         install={this.install.bind(this)}
         uninstall={this.uninstall.bind(this)}
@@ -109,9 +108,7 @@ export class FilterableListContainer<
 
   protected search(searchOptions: S): void {
     const { searchable } = this.props;
-    searchable
-      .search(searchOptions)
-      .then((items) => this.setState({ items: this.props.sort(items) }));
+    searchable.search(searchOptions).then((items) => this.setState({ items }));
   }
 
   protected async install(
@@ -127,7 +124,7 @@ export class FilterableListContainer<
       run: ({ progressId }) => install({ item, progressId, version }),
     });
     const items = await searchable.search(this.state.searchOptions);
-    this.setState({ items: this.props.sort(items) });
+    this.setState({ items });
   }
 
   protected async uninstall(item: T): Promise<void> {
@@ -155,7 +152,7 @@ export class FilterableListContainer<
       run: ({ progressId }) => uninstall({ item, progressId }),
     });
     const items = await searchable.search(this.state.searchOptions);
-    this.setState({ items: this.props.sort(items) });
+    this.setState({ items });
   }
 }
 
@@ -168,7 +165,6 @@ export namespace FilterableListContainer {
     readonly container: ListWidget<T, S>;
     readonly searchable: Searchable<T, S>;
     readonly itemLabel: (item: T) => string;
-    readonly itemDeprecated: (item: T) => boolean;
     readonly itemRenderer: ListItemRenderer<T>;
     readonly filterRenderer: FilterRenderer<S>;
     readonly resolveFocus: (element: HTMLElement | undefined) => void;
@@ -192,7 +188,6 @@ export namespace FilterableListContainer {
       progressId: string;
     }) => Promise<void>;
     readonly commandService: CommandService;
-    readonly sort: (items: T[]) => T[];
   }
 
   export interface State<T, S extends Searchable.Options> {

arduino-ide-extension/src/browser/widgets/component-list/list-widget.tsx

@@ -53,11 +53,9 @@ export abstract class ListWidget<
    */
   protected firstActivate = true;
 
-  protected readonly defaultSortComparator: (left: T, right: T) => number;
-
   constructor(protected options: ListWidget.Options<T, S>) {
     super();
-    const { id, label, iconClass, itemDeprecated, itemLabel } = options;
+    const { id, label, iconClass } = options;
     this.id = id;
     this.title.label = label;
     this.title.caption = label;
@@ -67,15 +65,6 @@ export abstract class ListWidget<
     this.node.tabIndex = 0; // To be able to set the focus on the widget.
     this.scrollOptions = undefined;
     this.toDispose.push(this.searchOptionsChangeEmitter);
-
-    this.defaultSortComparator = (left, right): number => {
-      // always put deprecated items at the bottom of the list
-      if (itemDeprecated(left)) {
-        return 1;
-      }
-
-      return itemLabel(left).localeCompare(itemLabel(right));
-    };
   }
 
   @postConstruct()
@@ -144,30 +133,6 @@ export abstract class ListWidget<
     return this.options.installable.uninstall({ item, progressId });
   }
 
-  protected filterableListSort = (items: T[]): T[] => {
-    const isArduinoTypeComparator = (left: T, right: T) => {
-      const aIsArduinoType = left.types.includes('Arduino');
-      const bIsArduinoType = right.types.includes('Arduino');
-
-      if (aIsArduinoType && !bIsArduinoType && !left.deprecated) {
-        return -1;
-      }
-
-      if (!aIsArduinoType && bIsArduinoType && !right.deprecated) {
-        return 1;
-      }
-
-      return 0;
-    };
-
-    return items.sort((left, right) => {
-      return (
-        isArduinoTypeComparator(left, right) ||
-        this.defaultSortComparator(left, right)
-      );
-    });
-  };
-
   render(): React.ReactNode {
     return (
       <FilterableListContainer<T, S>
@@ -178,14 +143,12 @@ export abstract class ListWidget<
         install={this.install.bind(this)}
         uninstall={this.uninstall.bind(this)}
         itemLabel={this.options.itemLabel}
-        itemDeprecated={this.options.itemDeprecated}
         itemRenderer={this.options.itemRenderer}
         filterRenderer={this.options.filterRenderer}
         searchOptionsDidChange={this.searchOptionsChangeEmitter.event}
         messageService={this.messageService}
         commandService={this.commandService}
         responseService={this.responseService}
-        sort={this.filterableListSort}
       />
     );
   }
@@ -218,7 +181,6 @@ export namespace ListWidget {
     readonly installable: Installable<T>;
     readonly searchable: Searchable<T, S>;
     readonly itemLabel: (item: T) => string;
-    readonly itemDeprecated: (item: T) => boolean;
     readonly itemRenderer: ListItemRenderer<T>;
     readonly filterRenderer: FilterRenderer<S>;
     readonly defaultSearchOptions: S;

arduino-ide-extension/src/common/protocol/arduino-component.ts

@@ -2,7 +2,7 @@ import { Installable } from './installable';
 
 export interface ArduinoComponent {
   readonly name: string;
-  readonly deprecated: boolean;
+  readonly deprecated?: boolean;
   readonly author: string;
   readonly summary: string;
   readonly description: string;

arduino-ide-extension/src/common/protocol/searchable.ts

@@ -1,4 +1,5 @@
 import URI from '@theia/core/lib/common/uri';
+import type { ArduinoComponent } from './arduino-component';
 
 export interface Searchable<T, O extends Searchable.Options> {
   search(options: O): Promise<T[]>;
@@ -31,3 +32,31 @@ export namespace Searchable {
     }
   }
 }
+
+// IDE2 must keep the library search order from the CLI but do additional boosting
+// https://github.com/arduino/arduino-ide/issues/1106
+// This additional search result boosting considers the following groups: 'Arduino', '', 'Arduino-Retired', and 'Retired'.
+// If two libraries fall into the same group, the original index is the tiebreaker.
+export type SortGroup = 'Arduino' | '' | 'Arduino-Retired' | 'Retired';
+const sortGroupOrder: Record<SortGroup, number> = {
+  Arduino: 0,
+  '': 1,
+  'Arduino-Retired': 2,
+  Retired: 3,
+};
+
+export function sortComponents<T extends ArduinoComponent>(
+  components: T[],
+  group: (component: T) => SortGroup
+): T[] {
+  return components
+    .map((component, index) => ({ ...component, index }))
+    .sort((left, right) => {
+      const leftGroup = group(left);
+      const rightGroup = group(right);
+      if (leftGroup === rightGroup) {
+        return left.index - right.index;
+      }
+      return sortGroupOrder[leftGroup] - sortGroupOrder[rightGroup];
+    });
+}

arduino-ide-extension/src/node/boards-service-impl.ts

@@ -17,6 +17,8 @@ import {
   BoardWithPackage,
   BoardUserField,
   BoardSearch,
+  sortComponents,
+  SortGroup,
 } from '../common/protocol';
 import {
   PlatformInstallRequest,
@@ -405,7 +407,8 @@ export class BoardsServiceImpl
     }
 
     const filter = this.typePredicate(options);
-    return [...packages.values()].filter(filter);
+    const boardsPackages = [...packages.values()].filter(filter);
+    return sortComponents(boardsPackages, boardsPackageSortGroup);
   }
 
   private typePredicate(
@@ -559,3 +562,14 @@ function isMissingPlatformError(error: unknown): boolean {
   }
   return false;
 }
+
+function boardsPackageSortGroup(boardsPackage: BoardsPackage): SortGroup {
+  const types: string[] = [];
+  if (boardsPackage.types.includes('Arduino')) {
+    types.push('Arduino');
+  }
+  if (boardsPackage.deprecated) {
+    types.push('Retired');
+  }
+  return types.join('-') as SortGroup;
+}

arduino-ide-extension/src/node/library-service-impl.ts

@@ -1,34 +1,39 @@
-import { injectable, inject } from '@theia/core/shared/inversify';
+import { ILogger, notEmpty } from '@theia/core';
+import { FileUri } from '@theia/core/lib/node';
+import { inject, injectable } from '@theia/core/shared/inversify';
+import { duration } from '../common/decorators';
+import {
+  NotificationServiceServer,
+  ResponseService,
+  sortComponents,
+  SortGroup,
+} from '../common/protocol';
+import { Installable } from '../common/protocol/installable';
 import {
   LibraryDependency,
   LibraryLocation,
   LibraryPackage,
   LibrarySearch,
   LibraryService,
 } from '../common/protocol/library-service';
-import { CoreClientAware } from './core-client-provider';
 import { BoardDiscovery } from './board-discovery';
 import {
   InstalledLibrary,
   Library,
+  LibraryInstallLocation,
   LibraryInstallRequest,
   LibraryListRequest,
   LibraryListResponse,
   LibraryLocation as GrpcLibraryLocation,
   LibraryRelease,
   LibraryResolveDependenciesRequest,
-  LibraryUninstallRequest,
-  ZipLibraryInstallRequest,
   LibrarySearchRequest,
   LibrarySearchResponse,
-  LibraryInstallLocation,
+  LibraryUninstallRequest,
+  ZipLibraryInstallRequest,
 } from './cli-protocol/cc/arduino/cli/commands/v1/lib_pb';
-import { Installable } from '../common/protocol/installable';
-import { ILogger, notEmpty } from '@theia/core';
-import { FileUri } from '@theia/core/lib/node';
-import { ResponseService, NotificationServiceServer } from '../common/protocol';
+import { CoreClientAware } from './core-client-provider';
 import { ExecuteWithProgress } from './grpc-progressible';
-import { duration } from '../common/decorators';
 
 @injectable()
 export class LibraryServiceImpl
@@ -108,7 +113,10 @@ export class LibraryServiceImpl
 
     const typePredicate = this.typePredicate(options);
     const topicPredicate = this.topicPredicate(options);
-    return items.filter((item) => typePredicate(item) && topicPredicate(item));
+    const libraries = items.filter(
+      (item) => typePredicate(item) && topicPredicate(item)
+    );
+    return sortComponents(libraries, librarySortGroup);
   }
 
   private typePredicate(
@@ -448,7 +456,6 @@ function toLibrary(
     name: '',
     exampleUris: [],
     installable: false,
-    deprecated: false,
     location: 0,
     ...pkg,
 
@@ -462,3 +469,14 @@ function toLibrary(
     types: lib.getTypesList(),
   };
 }
+
+// Libraries do not have a deprecated property. The deprecated information is inferred if 'Retired' is in 'types'
+function librarySortGroup(library: LibraryPackage): SortGroup {
+  const types: string[] = [];
+  for (const type of ['Arduino', 'Retired']) {
+    if (library.types.includes(type)) {
+      types.push(type);
+    }
+  }
+  return types.join('-') as SortGroup;
+}