Show user fields dialog again if upload fails (#1415)

* Show user fields dialog again if upload fails

* move user fields logic into own contribution

* apply suggestions

arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts

@@ -337,6 +337,7 @@ import { CheckForUpdates } from './contributions/check-for-updates';
 import { OutputEditorFactory } from './theia/output/output-editor-factory';
 import { StartupTaskProvider } from '../electron-common/startup-task';
 import { DeleteSketch } from './contributions/delete-sketch';
+import { UserFields } from './contributions/user-fields';
 
 const registerArduinoThemes = () => {
   const themes: MonacoThemeJson[] = [
@@ -761,6 +762,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   Contribution.configure(bind, OpenBoardsConfig);
   Contribution.configure(bind, SketchFilesTracker);
   Contribution.configure(bind, CheckForUpdates);
+  Contribution.configure(bind, UserFields);
   Contribution.configure(bind, DeleteSketch);
 
   bindContributionProvider(bind, StartupTaskProvider);

arduino-ide-extension/src/browser/contributions/upload-sketch.ts

@@ -1,7 +1,7 @@
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { Emitter } from '@theia/core/lib/common/event';
-import { BoardUserField, CoreService, Port } from '../../common/protocol';
+import { CoreService, Port } from '../../common/protocol';
-import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus';
+import { ArduinoMenus } from '../menu/arduino-menus';
 import { ArduinoToolbar } from '../toolbar/arduino-toolbar';
 import {
   Command,
@@ -11,96 +11,36 @@ import {
   TabBarToolbarRegistry,
   CoreServiceContribution,
 } from './contribution';
-import { UserFieldsDialog } from '../dialogs/user-fields/user-fields-dialog';
+import { deepClone, nls } from '@theia/core/lib/common';
-import { deepClone, DisposableCollection, nls } from '@theia/core/lib/common';
 import { CurrentSketch } from '../../common/protocol/sketches-service-client-impl';
 import type { VerifySketchParams } from './verify-sketch';
+import { UserFields } from './user-fields';
 
 @injectable()
 export class UploadSketch extends CoreServiceContribution {
-  @inject(MenuModelRegistry)
-  private readonly menuRegistry: MenuModelRegistry;
-
-  @inject(UserFieldsDialog)
-  private readonly userFieldsDialog: UserFieldsDialog;
-
-  private boardRequiresUserFields = false;
-  private readonly cachedUserFields: Map<string, BoardUserField[]> = new Map();
-  private readonly menuActionsDisposables = new DisposableCollection();
-
   private readonly onDidChangeEmitter = new Emitter<void>();
   private readonly onDidChange = this.onDidChangeEmitter.event;
   private uploadInProgress = false;
 
-  protected override init(): void {
+  @inject(UserFields)
-    super.init();
+  private readonly userFields: UserFields;
-    this.boardsServiceProvider.onBoardsConfigChanged(async () => {
-      const userFields =
-        await this.boardsServiceProvider.selectedBoardUserFields();
-      this.boardRequiresUserFields = userFields.length > 0;
-      this.registerMenus(this.menuRegistry);
-    });
-  }
-
-  private selectedFqbnAddress(): string {
-    const { boardsConfig } = this.boardsServiceProvider;
-    const fqbn = boardsConfig.selectedBoard?.fqbn;
-    if (!fqbn) {
-      return '';
-    }
-    const address =
-      boardsConfig.selectedBoard?.port?.address ||
-      boardsConfig.selectedPort?.address;
-    if (!address) {
-      return '';
-    }
-    return fqbn + '|' + address;
-  }
 
   override registerCommands(registry: CommandRegistry): void {
     registry.registerCommand(UploadSketch.Commands.UPLOAD_SKETCH, {
       execute: async () => {
-        const key = this.selectedFqbnAddress();
+        if (await this.userFields.checkUserFieldsDialog()) {
-        if (
+          this.uploadSketch();
-          this.boardRequiresUserFields &&
-          key &&
-          !this.cachedUserFields.has(key)
-        ) {
-          // Deep clone the array of board fields to avoid editing the cached ones
-          this.userFieldsDialog.value = (
-            await this.boardsServiceProvider.selectedBoardUserFields()
-          ).map((f) => ({ ...f }));
-          const result = await this.userFieldsDialog.open();
-          if (!result) {
-            return;
-          }
-          this.cachedUserFields.set(key, result);
         }
-        this.uploadSketch();
       },
       isEnabled: () => !this.uploadInProgress,
     });
     registry.registerCommand(UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION, {
       execute: async () => {
-        const key = this.selectedFqbnAddress();
+        if (await this.userFields.checkUserFieldsDialog(true)) {
-        if (!key) {
+          this.uploadSketch();
-          return;
         }
-
-        const cached = this.cachedUserFields.get(key);
-        // Deep clone the array of board fields to avoid editing the cached ones
-        this.userFieldsDialog.value = (
-          cached ?? (await this.boardsServiceProvider.selectedBoardUserFields())
-        ).map((f) => ({ ...f }));
-
-        const result = await this.userFieldsDialog.open();
-        if (!result) {
-          return;
-        }
-        this.cachedUserFields.set(key, result);
-        this.uploadSketch();
       },
-      isEnabled: () => !this.uploadInProgress && this.boardRequiresUserFields,
+      isEnabled: () => !this.uploadInProgress && this.userFields.isRequired(),
     });
     registry.registerCommand(
       UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER,
@@ -120,45 +60,20 @@ export class UploadSketch extends CoreServiceContribution {
   }
 
   override registerMenus(registry: MenuModelRegistry): void {
-    this.menuActionsDisposables.dispose();
+    registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
-    this.menuActionsDisposables.push(
+      commandId: UploadSketch.Commands.UPLOAD_SKETCH.id,
-      registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
+      label: nls.localize('arduino/sketch/upload', 'Upload'),
-        commandId: UploadSketch.Commands.UPLOAD_SKETCH.id,
+      order: '1',
-        label: nls.localize('arduino/sketch/upload', 'Upload'),
+    });
-        order: '1',
+
-      })
+    registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
-    );
+      commandId: UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER.id,
-    if (this.boardRequiresUserFields) {
+      label: nls.localize(
-      this.menuActionsDisposables.push(
+        'arduino/sketch/uploadUsingProgrammer',
-        registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
+        'Upload Using Programmer'
-          commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id,
+      ),
-          label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label,
+      order: '3',
-          order: '2',
+    });
-        })
-      );
-    } else {
-      this.menuActionsDisposables.push(
-        registry.registerMenuNode(
-          ArduinoMenus.SKETCH__MAIN_GROUP,
-          new PlaceholderMenuNode(
-            ArduinoMenus.SKETCH__MAIN_GROUP,
-            // commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id,
-            UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label,
-            { order: '2' }
-          )
-        )
-      );
-    }
-    this.menuActionsDisposables.push(
-      registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
-        commandId: UploadSketch.Commands.UPLOAD_SKETCH_USING_PROGRAMMER.id,
-        label: nls.localize(
-          'arduino/sketch/uploadUsingProgrammer',
-          'Upload Using Programmer'
-        ),
-        order: '3',
-      })
-    );
   }
 
   override registerKeybindings(registry: KeybindingRegistry): void {
@@ -215,18 +130,7 @@ export class UploadSketch extends CoreServiceContribution {
         return;
       }
 
-      // TODO: This does not belong here.
+      if (!this.userFields.checkUserFieldsForUpload()) {
-      // IDE2 should not do any preliminary checks but let the CLI fail and then toast a user consumable error message.
-      if (
-        uploadOptions.userFields.length === 0 &&
-        this.boardRequiresUserFields
-      ) {
-        this.messageService.error(
-          nls.localize(
-            'arduino/sketch/userFieldsNotFoundError',
-            "Can't find user fields for connected board"
-          )
-        );
         return;
       }
 
@@ -242,6 +146,7 @@ export class UploadSketch extends CoreServiceContribution {
         { timeout: 3000 }
       );
     } catch (e) {
+      this.userFields.notifyFailedWithError(e);
       this.handleError(e);
     } finally {
       this.uploadInProgress = false;
@@ -258,7 +163,7 @@ export class UploadSketch extends CoreServiceContribution {
     if (!CurrentSketch.isValid(sketch)) {
       return undefined;
     }
-    const userFields = this.userFields();
+    const userFields = this.userFields.getUserFields();
     const { boardsConfig } = this.boardsServiceProvider;
     const [fqbn, { selectedProgrammer: programmer }, verify, verbose] =
       await Promise.all([
@@ -301,10 +206,6 @@ export class UploadSketch extends CoreServiceContribution {
     return port;
   }
 
-  private userFields(): BoardUserField[] {
-    return this.cachedUserFields.get(this.selectedFqbnAddress()) ?? [];
-  }
-
   /**
    * Converts the `VENDOR:ARCHITECTURE:BOARD_ID[:MENU_ID=OPTION_ID[,MENU2_ID=OPTION_ID ...]]` FQBN to
    * `VENDOR:ARCHITECTURE:BOARD_ID` format.

arduino-ide-extension/src/browser/contributions/user-fields.ts

@@ -0,0 +1,150 @@
+import { inject, injectable } from '@theia/core/shared/inversify';
+import { DisposableCollection, nls } from '@theia/core/lib/common';
+import { BoardUserField, CoreError } from '../../common/protocol';
+import { BoardsServiceProvider } from '../boards/boards-service-provider';
+import { UserFieldsDialog } from '../dialogs/user-fields/user-fields-dialog';
+import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus';
+import { MenuModelRegistry, Contribution } from './contribution';
+import { UploadSketch } from './upload-sketch';
+
+@injectable()
+export class UserFields extends Contribution {
+  private boardRequiresUserFields = false;
+  private userFieldsSet = false;
+  private readonly cachedUserFields: Map<string, BoardUserField[]> = new Map();
+  private readonly menuActionsDisposables = new DisposableCollection();
+
+  @inject(UserFieldsDialog)
+  private readonly userFieldsDialog: UserFieldsDialog;
+
+  @inject(BoardsServiceProvider)
+  private readonly boardsServiceProvider: BoardsServiceProvider;
+
+  @inject(MenuModelRegistry)
+  private readonly menuRegistry: MenuModelRegistry;
+
+  protected override init(): void {
+    super.init();
+    this.boardsServiceProvider.onBoardsConfigChanged(async () => {
+      const userFields =
+        await this.boardsServiceProvider.selectedBoardUserFields();
+      this.boardRequiresUserFields = userFields.length > 0;
+      this.registerMenus(this.menuRegistry);
+    });
+  }
+
+  override registerMenus(registry: MenuModelRegistry): void {
+    this.menuActionsDisposables.dispose();
+    if (this.boardRequiresUserFields) {
+      this.menuActionsDisposables.push(
+        registry.registerMenuAction(ArduinoMenus.SKETCH__MAIN_GROUP, {
+          commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id,
+          label: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label,
+          order: '2',
+        })
+      );
+    } else {
+      this.menuActionsDisposables.push(
+        registry.registerMenuNode(
+          ArduinoMenus.SKETCH__MAIN_GROUP,
+          new PlaceholderMenuNode(
+            ArduinoMenus.SKETCH__MAIN_GROUP,
+            // commandId: UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.id,
+            UploadSketch.Commands.UPLOAD_WITH_CONFIGURATION.label,
+            { order: '2' }
+          )
+        )
+      );
+    }
+  }
+
+  private selectedFqbnAddress(): string | undefined  {
+    const { boardsConfig } = this.boardsServiceProvider;
+    const fqbn = boardsConfig.selectedBoard?.fqbn;
+    if (!fqbn) {
+      return undefined;
+    }
+    const address =
+      boardsConfig.selectedBoard?.port?.address ||
+      boardsConfig.selectedPort?.address;
+    if (!address) {
+      return undefined;
+    }
+    return fqbn + '|' + address;
+  }
+
+  private async showUserFieldsDialog(
+    key: string
+  ): Promise<BoardUserField[] | undefined> {
+    const cached = this.cachedUserFields.get(key);
+    // Deep clone the array of board fields to avoid editing the cached ones
+    this.userFieldsDialog.value = cached ? cached.slice() : await this.boardsServiceProvider.selectedBoardUserFields();
+    const result = await this.userFieldsDialog.open();
+    if (!result) {
+      return;
+    }
+
+    this.userFieldsSet = true;
+    this.cachedUserFields.set(key, result);
+    return result;
+  }
+
+  async checkUserFieldsDialog(forceOpen = false): Promise<boolean> {
+    const key = this.selectedFqbnAddress();
+    if (!key) {
+      return false;
+    }
+    /*
+      If the board requires to be configured with user fields, we want
+      to show the user fields dialog, but only if they weren't already
+      filled in or if they were filled in, but the previous upload failed.
+    */
+    if (
+      !forceOpen &&
+      (!this.boardRequiresUserFields ||
+        (this.cachedUserFields.has(key) && this.userFieldsSet))
+    ) {
+      return true;
+    }
+    const userFieldsFilledIn = Boolean(await this.showUserFieldsDialog(key));
+    return userFieldsFilledIn;
+  }
+
+  checkUserFieldsForUpload(): boolean {
+    // TODO: This does not belong here.
+    // IDE2 should not do any preliminary checks but let the CLI fail and then toast a user consumable error message.
+    if (!this.boardRequiresUserFields || this.getUserFields().length > 0) {
+      this.userFieldsSet = true;
+      return true;
+    }
+    this.messageService.error(
+      nls.localize(
+        'arduino/sketch/userFieldsNotFoundError',
+        "Can't find user fields for connected board"
+      )
+    );
+    this.userFieldsSet = false;
+    return false;
+  }
+
+  getUserFields(): BoardUserField[] {
+    const fqbnAddress = this.selectedFqbnAddress();
+    if (!fqbnAddress) {
+      return [];
+    }
+    return this.cachedUserFields.get(fqbnAddress) ?? [];
+  }
+
+  isRequired(): boolean {
+    return this.boardRequiresUserFields;
+  }
+
+  notifyFailedWithError(e: Error): void {
+    if (
+      this.boardRequiresUserFields &&
+      CoreError.UploadFailed.is(e)
+    ) {
+      this.userFieldsSet = false;
+    }
+  }
+}