fix: enforce valid sketch folder name on Save as

Closes #1599

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

Diff for: arduino-ide-extension/src/browser/contributions/new-cloud-sketch.ts

@@ -30,7 +30,13 @@ import { CloudSketchbookTreeWidget } from '../widgets/cloud-sketchbook/cloud-ske
 import { SketchbookCommands } from '../widgets/sketchbook/sketchbook-commands';
 import { SketchbookWidget } from '../widgets/sketchbook/sketchbook-widget';
 import { SketchbookWidgetContribution } from '../widgets/sketchbook/sketchbook-widget-contribution';
-import { Command, CommandRegistry, Contribution, URI } from './contribution';
+import {
+  Command,
+  CommandRegistry,
+  Contribution,
+  Sketch,
+  URI,
+} from './contribution';
 
 @injectable()
 export class NewCloudSketch extends Contribution {
@@ -234,14 +240,7 @@ export class NewCloudSketch extends Contribution {
               input
             );
           }
-          // This is how https://create.arduino.cc/editor/ works when renaming a sketch.
-          if (/^[0-9a-zA-Z_]{1,36}$/.test(input)) {
-            return '';
-          }
-          return nls.localize(
-            'arduino/newCloudSketch/invalidSketchName',
-            'The name must consist of basic letters, numbers, or underscores. The maximum length is 36 characters.'
-          );
+          return Sketch.validateCloudSketchFolderName(input) ?? '';
         },
       },
       this.labelProvider,

Diff for: arduino-ide-extension/src/browser/contributions/save-as-sketch.ts

@@ -1,5 +1,8 @@
 import { inject, injectable } from '@theia/core/shared/inversify';
-import * as remote from '@theia/core/electron-shared/@electron/remote';
+import {
+  dialog,
+  getCurrentWindow,
+} from '@theia/core/electron-shared/@electron/remote';
 import * as dateFormat from 'dateformat';
 import { ArduinoMenus } from '../menu/arduino-menus';
 import {
@@ -9,9 +12,14 @@ import {
   CommandRegistry,
   MenuModelRegistry,
   KeybindingRegistry,
+  Sketch,
 } from './contribution';
 import { nls } from '@theia/core/lib/common';
-import { ApplicationShell, NavigatableWidget, Saveable } from '@theia/core/lib/browser';
+import {
+  ApplicationShell,
+  NavigatableWidget,
+  Saveable,
+} from '@theia/core/lib/browser';
 import { WindowService } from '@theia/core/lib/browser/window/window-service';
 import { CurrentSketch } from '../../common/protocol/sketches-service-client-impl';
 import { WorkspaceInput } from '@theia/workspace/lib/browser';
@@ -90,28 +98,19 @@ export class SaveAsSketch extends SketchContribution {
         : sketch.name
     );
     const defaultPath = await this.fileService.fsPath(defaultUri);
-    const { filePath, canceled } = await remote.dialog.showSaveDialog(
-      remote.getCurrentWindow(),
-      {
-        title: nls.localize(
-          'arduino/sketch/saveFolderAs',
-          'Save sketch folder as...'
-        ),
-        defaultPath,
-      }
-    );
-    if (!filePath || canceled) {
-      return false;
-    }
-    const destinationUri = await this.fileSystemExt.getUri(filePath);
+    const destinationUri = await this.promptForSketchFolderName(defaultPath);
     if (!destinationUri) {
       return false;
     }
     const workspaceUri = await this.sketchService.copy(sketch, {
       destinationUri,
     });
     if (workspaceUri) {
-      await this.saveOntoCopiedSketch(sketch.mainFileUri, sketch.uri, workspaceUri);
+      await this.saveOntoCopiedSketch(
+        sketch.mainFileUri,
+        sketch.uri,
+        workspaceUri
+      );
       if (markAsRecentlyOpened) {
         this.sketchService.markAsRecentlyOpened(workspaceUri);
       }
@@ -133,6 +132,62 @@ export class SaveAsSketch extends SketchContribution {
     return !!workspaceUri;
   }
 
+  /**
+   * Prompts for the new sketch folder name until a valid one is give,
+   * then resolves with the destination sketch folder URI string,
+   * or `undefined` if the operation was canceled.
+   */
+  private async promptForSketchFolderName(
+    defaultPath: string
+  ): Promise<string | undefined> {
+    let sketchFolderDestinationUri: string | undefined;
+    while (!sketchFolderDestinationUri) {
+      const { filePath } = await dialog.showSaveDialog(getCurrentWindow(), {
+        title: nls.localize(
+          'arduino/sketch/saveFolderAs',
+          'Save sketch folder as...'
+        ),
+        defaultPath,
+      });
+      if (!filePath) {
+        return undefined;
+      }
+      const destinationUri = await this.fileSystemExt.getUri(filePath);
+      const sketchFolderName = new URI(destinationUri).path.base;
+      const errorMessage = Sketch.validateSketchFolderName(sketchFolderName);
+      if (errorMessage) {
+        const message = `
+${nls.localize(
+  'arduino/sketch/invalidSketchFolderNameTitle',
+  "Invalid sketch folder name: '{0}'",
+  sketchFolderName
+)}
+
+${errorMessage}
+
+${nls.localize(
+  'arduino/sketch/editInvalidSketchFolderName',
+  'Do you want to try to save the sketch folder with a different name?'
+)}`.trim();
+        defaultPath = filePath;
+        const { response } = await dialog.showMessageBox(getCurrentWindow(), {
+          message,
+          buttons: [
+            nls.localize('vscode/issueMainService/cancel', 'Cancel'),
+            nls.localize('vscode/extensionsUtils/yes', 'Yes'),
+          ],
+        });
+        // cancel
+        if (response === 0) {
+          return undefined;
+        }
+      } else {
+        sketchFolderDestinationUri = destinationUri;
+      }
+    }
+    return sketchFolderDestinationUri;
+  }
+
   private async saveOntoCopiedSketch(mainFileUri: string, sketchUri: string, newSketchUri: string): Promise<void> {
     const widgets = this.applicationShell.widgets;
     const snapshots = new Map<string, object>();

Diff for: arduino-ide-extension/src/browser/contributions/sketchbook.ts

@@ -1,14 +1,24 @@
-import { injectable } from '@theia/core/shared/inversify';
+import { dialog } from '@theia/core/electron-shared/@electron/remote';
 import { CommandHandler } from '@theia/core/lib/common/command';
-import { MenuModelRegistry } from './contribution';
+import { nls } from '@theia/core/lib/common/nls';
+import { inject, injectable } from '@theia/core/shared/inversify';
+import { SketchContainer, SketchesError } from '../../common/protocol';
 import { ArduinoMenus } from '../menu/arduino-menus';
+import { WindowServiceExt } from '../theia/core/window-service-ext';
+import { MenuModelRegistry, URI } from './contribution';
 import { Examples } from './examples';
-import { SketchContainer, SketchesError } from '../../common/protocol';
 import { OpenSketch } from './open-sketch';
-import { nls } from '@theia/core/lib/common/nls';
 
 @injectable()
 export class Sketchbook extends Examples {
+  @inject(WindowServiceExt)
+  private readonly windowService: WindowServiceExt;
+
+  /**
+   * Prompt invalid sketch folder names only once per sketchbook.
+   */
+  private _sketchbookUriOfInvalidSketches: string | undefined;
+
   override onStart(): void {
     this.sketchServiceClient.onSketchbookDidChange(() => this.update());
     this.configService.onDidChangeSketchDirUri(() => this.update());
@@ -19,10 +29,77 @@ export class Sketchbook extends Examples {
   }
 
   protected override update(): void {
-    this.sketchService.getSketches({}).then((container) => {
-      this.register(container);
-      this.menuManager.update();
-    });
+    this.sketchService
+      .getSketches({})
+      .then(({ container, sketchesWithInvalidFolderName }) => {
+        const currentSketchbookUri = this.configService
+          .tryGetSketchDirUri()
+          ?.toString();
+        if (
+          currentSketchbookUri &&
+          currentSketchbookUri !== this._sketchbookUriOfInvalidSketches
+        ) {
+          this._sketchbookUriOfInvalidSketches = currentSketchbookUri;
+          // Logs invalid sketch folder names once per sketchbook
+          // If sketchbook changes, logs the invalid sketch folder names, if any, again.
+          nls.localize(
+            'arduino/sketch/ignoringSketchWithBadNameMessage',
+            'The sketch "{0}" cannot be used.\nSketch names must contain only basic letters and numbers\n(ASCII-only with no spaces, and it cannot start with a number).\nTo get rid of this message, remove the sketch from\n{1}',
+            'name',
+            'path'
+          );
+          if (sketchesWithInvalidFolderName.length) {
+            this.windowService
+              .isPrimaryWindow()
+              .then(async (isLastFocusedWindow) => {
+                if (!isLastFocusedWindow) {
+                  return;
+                }
+                const dialogInputs = await Promise.all(
+                  sketchesWithInvalidFolderName.map(async (ref) => {
+                    const fsPath = await this.fileService.fsPath(
+                      new URI(ref.uri)
+                    );
+                    return {
+                      name: ref.name,
+                      path: fsPath,
+                    };
+                  })
+                );
+                dialogInputs.forEach(({ name, path }) =>
+                  dialog.showErrorBox(
+                    nls.localize(
+                      'arduino/sketch/ignoringSketchWithBadNameTitle',
+                      'Ignoring sketch with bad name'
+                    ),
+                    `
+${nls.localize(
+  'arduino/sketch/ignoringSketchWithBadNameMessage0',
+  'The sketch "{0}" cannot be used.',
+  name
+)}
+${nls.localize(
+  'arduino/sketch/ignoringSketchWithBadNameMessage1',
+  'Sketch names must contain only basic letters and numbers'
+)}
+${nls.localize(
+  'arduino/sketch/ignoringSketchWithBadNameMessage2',
+  '(ASCII-only with no spaces, and it cannot start with a number).'
+)}
+${nls.localize(
+  'arduino/sketch/ignoringSketchWithBadNameMessage3',
+  'To get rid of this message, remove the sketch from'
+)}
+${path}
+`.trim()
+                  )
+                );
+              });
+          }
+        }
+        this.register(container);
+        this.menuManager.update();
+      });
   }
 
   override registerMenus(registry: MenuModelRegistry): void {

Diff for: arduino-ide-extension/src/browser/theia/core/window-service-ext.ts

@@ -6,5 +6,10 @@ export interface WindowServiceExt {
    * Returns with a promise that resolves to `true` if the current window is the first window.
    */
   isFirstWindow(): Promise<boolean>;
+  /**
+   * If the first window is still open and the caller is the first window, it is `true`. Otherwise, `false`.
+   * If the first window is closed, calling this method will be `true` only for the window which has last focused.
+   */
+  isPrimaryWindow(): Promise<boolean>;
   reload(options?: StartupTask.Owner): void;
 }

Diff for: arduino-ide-extension/src/common/protocol/sketches-service-client-impl.ts

@@ -90,7 +90,7 @@ export class SketchesServiceClientImpl
     if (!sketchDirUri) {
       return;
     }
-    const container = await this.sketchService.getSketches({
+    const { container } = await this.sketchService.getSketches({
       uri: sketchDirUri.toString(),
     });
     for (const sketch of SketchContainer.toArray(container)) {

Diff for: arduino-ide-extension/src/common/protocol/sketches-service.ts

@@ -1,4 +1,5 @@
 import { ApplicationError } from '@theia/core/lib/common/application-error';
+import { nls } from '@theia/core/lib/common/nls';
 import URI from '@theia/core/lib/common/uri';
 
 export namespace SketchesError {
@@ -31,9 +32,14 @@ export const SketchesService = Symbol('SketchesService');
 export interface SketchesService {
   /**
    * Resolves to a sketch container representing the hierarchical structure of the sketches.
-   * If `uri` is not given, `directories.user` will be user instead.
+   * If `uri` is not given, `directories.user` will be user instead. The `sketchesWithInvalidFolderName`
+   * array might contain sketches that were discovered, but due to their invalid name they were removed
+   * from the `container`.
    */
-  getSketches({ uri }: { uri?: string }): Promise<SketchContainer>;
+  getSketches({ uri }: { uri?: string }): Promise<{
+    container: SketchContainer;
+    sketchesWithInvalidFolderName: SketchRef[];
+  }>;
 
   /**
    * This is the TS implementation of `SketchLoad` from the CLI and should be replaced with a gRPC call eventually.
@@ -140,6 +146,50 @@ export interface Sketch extends SketchRef {
   readonly rootFolderFileUris: string[]; // `RootFolderFiles` (does not include the main sketch file)
 }
 export namespace Sketch {
+  export const invalidSketchFolderNameMessage = nls.localize(
+    'arduino/sketch/invalidSketchName',
+    'Sketch names must start with a letter or number, followed by letters, numbers, dashes, dots and underscores. Maximum length is 63 characters.'
+  );
+  const invalidCloudSketchFolderNameMessage = nls.localize(
+    'arduino/sketch/invalidCloudSketchName',
+    'The name must consist of basic letters, numbers, or underscores. The maximum length is 36 characters.'
+  );
+  /**
+   * `undefined` if the candidate sketch name is valid. Otherwise, the validation error message.
+   * Based on the [specs](https://arduino.github.io/arduino-cli/latest/sketch-specification/#sketch-folders-and-files).
+   */
+  export function validateSketchFolderName(
+    candidate: string
+  ): string | undefined {
+    return /^[0-9a-zA-Z]{1}[0-9a-zA-Z_\.-]{0,62}$/.test(candidate)
+      ? undefined
+      : invalidSketchFolderNameMessage;
+  }
+
+  /**
+   * `undefined` if the candidate sketch name is valid. Otherwise, the validation error message.
+   * Based on how https://create.arduino.cc/editor/ works.
+   */
+  export function validateCloudSketchFolderName(
+    candidate: string
+  ): string | undefined {
+    return /^[0-9a-zA-Z_]{1,36}$/.test(candidate)
+      ? undefined
+      : invalidCloudSketchFolderNameMessage;
+  }
+
+  /**
+   * Transforms the valid local sketch name into a valid cloud sketch name by replacing dots and dashes with underscore and trimming the length after 36 characters.
+   * Throws an error if `candidate` is not valid.
+   */
+  export function toValidCloudSketchFolderName(candidate: string): string {
+    const errorMessage = validateSketchFolderName(candidate);
+    if (errorMessage) {
+      throw new Error(errorMessage);
+    }
+    return candidate.replace(/\./g, '_').replace(/-/g, '_').slice(0, 36);
+  }
+
   export function is(arg: unknown): arg is Sketch {
     if (!SketchRef.is(arg)) {
       return false;

Diff for: arduino-ide-extension/src/electron-browser/theia/core/electron-window-service.ts

@@ -77,6 +77,11 @@ export class ElectronWindowService
     return this._firstWindow.promise;
   }
 
+  async isPrimaryWindow(): Promise<boolean> {
+    const windowId = remote.getCurrentWindow().id;
+    return this.mainWindowServiceExt.isPrimaryWindow(windowId);
+  }
+
   // Overridden because the default Theia implementation destroys the additional properties of the `options` arg, such as `tasks`.
   override openNewWindow(url: string, options?: NewWindowOptions): undefined {
     return this.delegate.openNewWindow(url, options);

Diff for: arduino-ide-extension/src/electron-common/electron-main-window-service-ext.ts

@@ -4,4 +4,5 @@ export const ElectronMainWindowServiceExt = Symbol(
 );
 export interface ElectronMainWindowServiceExt {
   isFirstWindow(windowId: number): Promise<boolean>;
+  isPrimaryWindow(windowId: number): Promise<boolean>;
 }