Fix boards listing (#1520)

* Fix boards listing * use arduio-cli sorting fix * re-use code to handle board list response * change `handleListBoards` visibility to `private` * pad menu items order with leading zeros to fix alphanumeric order
arduino · Oct 17, 2022 · 960a2d0 · 960a2d0
1 parent e577de4
commit 960a2d0
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 16 deletions.
diff --git a/arduino-ide-extension/src/browser/boards/boards-data-menu-updater.ts b/arduino-ide-extension/src/browser/boards/boards-data-menu-updater.ts
@@ -111,7 +111,7 @@ export class BoardsDataMenuUpdater implements FrontendApplicationContribution {
                   const { label } = commands.get(commandId)!;
                   this.menuRegistry.registerMenuAction(menuPath, {
                     commandId,
-                    order: `${i}`,
+                    order: String(i).padStart(4),
                     label,
                   });
                   return Disposable.create(() =>

diff --git a/arduino-ide-extension/src/browser/contributions/board-selection.ts b/arduino-ide-extension/src/browser/contributions/board-selection.ts
@@ -199,14 +199,15 @@ PID: ${PID}`;
     });
 
     // Installed boards
-    for (const board of installedBoards) {
+    installedBoards.forEach((board, index) => {
       const { packageId, packageName, fqbn, name, manuallyInstalled } = board;
 
       const packageLabel =
         packageName +
-        `${manuallyInstalled
-          ? nls.localize('arduino/board/inSketchbook', ' (in Sketchbook)')
-          : ''
+        `${
+          manuallyInstalled
+            ? nls.localize('arduino/board/inSketchbook', ' (in Sketchbook)')
+            : ''
         }`;
       // Platform submenu
       const platformMenuPath = [...boardsPackagesGroup, packageId];
@@ -239,14 +240,18 @@ PID: ${PID}`;
       };
 
       // Board menu
-      const menuAction = { commandId: id, label: name };
+      const menuAction = {
+        commandId: id,
+        label: name,
+        order: String(index).padStart(4), // pads with leading zeros for alphanumeric sort where order is 1, 2, 11, and NOT 1, 11, 2
+      };
       this.commandRegistry.registerCommand(command, handler);
       this.toDisposeBeforeMenuRebuild.push(
         Disposable.create(() => this.commandRegistry.unregisterCommand(command))
       );
       this.menuModelRegistry.registerMenuAction(platformMenuPath, menuAction);
       // Note: we do not dispose the menu actions individually. Calling `unregisterSubmenu` on the parent will wipe the children menu nodes recursively.
-    }
+    });
 
     // Installed ports
     const registerPorts = (
@@ -282,11 +287,13 @@ PID: ${PID}`;
 
       // First we show addresses with recognized boards connected,
       // then all the rest.
-      const sortedIDs = Object.keys(ports).sort((left: string, right: string): number => {
-        const [, leftBoards] = ports[left];
-        const [, rightBoards] = ports[right];
-        return rightBoards.length - leftBoards.length;
-      });
+      const sortedIDs = Object.keys(ports).sort(
+        (left: string, right: string): number => {
+          const [, leftBoards] = ports[left];
+          const [, rightBoards] = ports[right];
+          return rightBoards.length - leftBoards.length;
+        }
+      );
 
       for (let i = 0; i < sortedIDs.length; i++) {
         const portID = sortedIDs[i];
@@ -322,7 +329,7 @@ PID: ${PID}`;
         const menuAction = {
           commandId: id,
           label,
-          order: `${protocolOrder + i + 1}`,
+          order: String(protocolOrder + i + 1).padStart(4),
         };
         this.commandRegistry.registerCommand(command, handler);
         this.toDisposeBeforeMenuRebuild.push(
@@ -354,7 +361,7 @@ PID: ${PID}`;
   }
 
   protected async installedBoards(): Promise<InstalledBoardWithPackage[]> {
-    const allBoards = await this.boardsService.searchBoards({});
+    const allBoards = await this.boardsService.getInstalledBoards();
     return allBoards.filter(InstalledBoardWithPackage.is);
   }
 }

diff --git a/arduino-ide-extension/src/browser/contributions/sketch-control.ts b/arduino-ide-extension/src/browser/contributions/sketch-control.ts
@@ -176,7 +176,7 @@ export class SketchControl extends SketchContribution {
               {
                 commandId: command.id,
                 label: this.labelProvider.getName(uri),
-                order: `${i}`,
+                order: String(i).padStart(4),
               }
             );
             this.toDisposeBeforeCreateNewContextMenu.push(

diff --git a/arduino-ide-extension/src/common/protocol/boards-service.ts b/arduino-ide-extension/src/common/protocol/boards-service.ts
@@ -148,6 +148,7 @@ export interface BoardsService
     fqbn: string;
   }): Promise<BoardsPackage | undefined>;
   searchBoards({ query }: { query?: string }): Promise<BoardWithPackage[]>;
+  getInstalledBoards(): Promise<BoardWithPackage[]>;
   getBoardUserFields(options: {
     fqbn: string;
     protocol: string;

diff --git a/arduino-ide-extension/src/node/boards-service-impl.ts b/arduino-ide-extension/src/node/boards-service-impl.ts
@@ -32,6 +32,8 @@ import { CoreClientAware } from './core-client-provider';
 import {
   BoardDetailsRequest,
   BoardDetailsResponse,
+  BoardListAllRequest,
+  BoardListAllResponse,
   BoardSearchRequest,
 } from './cli-protocol/cc/arduino/cli/commands/v1/board_pb';
 import {
@@ -199,8 +201,28 @@ export class BoardsServiceImpl
     const req = new BoardSearchRequest();
     req.setSearchArgs(query || '');
     req.setInstance(instance);
+    return this.handleListBoards(client.boardSearch.bind(client), req);
+  }
+
+  async getInstalledBoards(): Promise<BoardWithPackage[]> {
+    const { instance, client } = await this.coreClient;
+    const req = new BoardListAllRequest();
+    req.setInstance(instance);
+    return this.handleListBoards(client.boardListAll.bind(client), req);
+  }
+
+  private async handleListBoards(
+    getBoards: (
+      request: BoardListAllRequest | BoardSearchRequest,
+      callback: (
+        error: ServiceError | null,
+        response: BoardListAllResponse
+      ) => void
+    ) => void,
+    request: BoardListAllRequest | BoardSearchRequest
+  ): Promise<BoardWithPackage[]> {
     const boards = await new Promise<BoardWithPackage[]>((resolve, reject) => {
-      client.boardSearch(req, (error, resp) => {
+      getBoards(request, (error, resp) => {
         if (error) {
           reject(error);
           return;