Skip to content

Commit 2cb9d64

Browse files
author
Akos Kitta
committed
fix: enforce valid sketch folder name on Save as
Closes #1599 Signed-off-by: Akos Kitta <[email protected]>
1 parent 692f29f commit 2cb9d64

File tree

13 files changed

+433
-71
lines changed

13 files changed

+433
-71
lines changed

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

+8-9
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@ import { CloudSketchbookTreeWidget } from '../widgets/cloud-sketchbook/cloud-ske
3030
import { SketchbookCommands } from '../widgets/sketchbook/sketchbook-commands';
3131
import { SketchbookWidget } from '../widgets/sketchbook/sketchbook-widget';
3232
import { SketchbookWidgetContribution } from '../widgets/sketchbook/sketchbook-widget-contribution';
33-
import { Command, CommandRegistry, Contribution, URI } from './contribution';
33+
import {
34+
Command,
35+
CommandRegistry,
36+
Contribution,
37+
Sketch,
38+
URI,
39+
} from './contribution';
3440

3541
@injectable()
3642
export class NewCloudSketch extends Contribution {
@@ -234,14 +240,7 @@ export class NewCloudSketch extends Contribution {
234240
input
235241
);
236242
}
237-
// This is how https://create.arduino.cc/editor/ works when renaming a sketch.
238-
if (/^[0-9a-zA-Z_]{1,36}$/.test(input)) {
239-
return '';
240-
}
241-
return nls.localize(
242-
'arduino/newCloudSketch/invalidSketchName',
243-
'The name must consist of basic letters, numbers, or underscores. The maximum length is 36 characters.'
244-
);
243+
return Sketch.validateCloudSketchFolderName(input) ?? '';
245244
},
246245
},
247246
this.labelProvider,

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

+64-13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as remote from '@theia/core/electron-shared/@electron/remote';
33
import * as dateFormat from 'dateformat';
44
import { ArduinoMenus } from '../menu/arduino-menus';
55
import {
6+
Sketch,
67
SketchContribution,
78
URI,
89
Command,
@@ -90,20 +91,9 @@ export class SaveAsSketch extends SketchContribution {
9091
: sketch.name
9192
);
9293
const defaultPath = await this.fileService.fsPath(defaultUri);
93-
const { filePath, canceled } = await remote.dialog.showSaveDialog(
94-
remote.getCurrentWindow(),
95-
{
96-
title: nls.localize(
97-
'arduino/sketch/saveFolderAs',
98-
'Save sketch folder as...'
99-
),
100-
defaultPath,
101-
}
94+
const destinationUri = await this.promptSketchFolderDestination(
95+
defaultPath
10296
);
103-
if (!filePath || canceled) {
104-
return false;
105-
}
106-
const destinationUri = await this.fileSystemExt.getUri(filePath);
10797
if (!destinationUri) {
10898
return false;
10999
}
@@ -133,6 +123,67 @@ export class SaveAsSketch extends SketchContribution {
133123
return !!workspaceUri;
134124
}
135125

126+
/**
127+
* Prompts for the new sketch folder name until a valid one is give,
128+
* then resolves with the destination sketch folder URI string,
129+
* or `undefined` if the operation was canceled.
130+
*/
131+
private async promptSketchFolderDestination(
132+
defaultPath: string
133+
): Promise<string | undefined> {
134+
let sketchFolderDestinationUri: string | undefined;
135+
while (!sketchFolderDestinationUri) {
136+
const { filePath } = await remote.dialog.showSaveDialog(
137+
remote.getCurrentWindow(),
138+
{
139+
title: nls.localize(
140+
'arduino/sketch/saveFolderAs',
141+
'Save sketch folder as...'
142+
),
143+
defaultPath,
144+
}
145+
);
146+
if (!filePath) {
147+
return undefined;
148+
}
149+
const destinationUri = await this.fileSystemExt.getUri(filePath);
150+
const sketchFolderName = new URI(destinationUri).path.base;
151+
const errorMessage = Sketch.validateSketchFolderName(sketchFolderName);
152+
if (errorMessage) {
153+
const message = `
154+
${nls.localize(
155+
'arduino/sketch/invalidSketchFolderNameTitle',
156+
"Invalid sketch folder name: '{0}'",
157+
sketchFolderName
158+
)}
159+
160+
${errorMessage}
161+
162+
${nls.localize(
163+
'arduino/sketch/editInvalidSketchFolderName',
164+
'Do you want to try to save the sketch folder with a different name?'
165+
)}`.trim();
166+
defaultPath = filePath;
167+
const { response } = await remote.dialog.showMessageBox(
168+
remote.getCurrentWindow(),
169+
{
170+
message,
171+
buttons: [
172+
nls.localize('vscode/issueMainService/cancel', 'Cancel'),
173+
nls.localize('vscode/extensionsUtils/yes', 'Yes'),
174+
],
175+
});
176+
// cancel
177+
if (response === 0) {
178+
return undefined;
179+
}
180+
} else {
181+
sketchFolderDestinationUri = destinationUri;
182+
}
183+
}
184+
return sketchFolderDestinationUri;
185+
}
186+
136187
private async saveOntoCopiedSketch(mainFileUri: string, sketchUri: string, newSketchUri: string): Promise<void> {
137188
const widgets = this.applicationShell.widgets;
138189
const snapshots = new Map<string, object>();

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

+94-10
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,112 @@
1-
import { injectable } from '@theia/core/shared/inversify';
2-
import { CommandHandler } from '@theia/core/lib/common/command';
3-
import { MenuModelRegistry } from './contribution';
1+
import * as remote from '@theia/core/electron-shared/@electron/remote';
2+
import type { CommandHandler } from '@theia/core/lib/common/command';
3+
import { nls } from '@theia/core/lib/common/nls';
4+
import { waitForEvent } from '@theia/core/lib/common/promise-util';
5+
import { inject, injectable } from '@theia/core/shared/inversify';
6+
import { SketchContainer, SketchesError } from '../../common/protocol';
47
import { ArduinoMenus } from '../menu/arduino-menus';
8+
import { WindowServiceExt } from '../theia/core/window-service-ext';
9+
import { MenuModelRegistry, URI } from './contribution';
510
import { Examples } from './examples';
6-
import { SketchContainer, SketchesError } from '../../common/protocol';
711
import { OpenSketch } from './open-sketch';
8-
import { nls } from '@theia/core/lib/common/nls';
912

1013
@injectable()
1114
export class Sketchbook extends Examples {
15+
@inject(WindowServiceExt)
16+
private readonly windowService: WindowServiceExt;
17+
private _currentSketchbookPath: string | undefined;
18+
1219
override onStart(): void {
1320
this.sketchServiceClient.onSketchbookDidChange(() => this.update());
1421
this.configService.onDidChangeSketchDirUri(() => this.update());
22+
// If this window is changing the settings, and sketchbook location is included in the changeset,
23+
// then the users must be warned about the invalid sketches from the new sketchbook.
24+
this.settingsService.onWillChangeSoon(async (unsavedSettings) => {
25+
// The sketchbook location is about to change.
26+
if (unsavedSettings.sketchbookPath !== this._currentSketchbookPath) {
27+
// Listen on both the settings and sketchbook location did change events
28+
const timeout = 5_000;
29+
const results = await Promise.allSettled([
30+
waitForEvent(this.settingsService.onDidChange, timeout),
31+
waitForEvent(this.configService.onDidChangeSketchDirUri, timeout),
32+
]);
33+
if (results.every(({ status }) => status === 'fulfilled')) {
34+
this.doUpdate(true);
35+
} else {
36+
console.warn(
37+
'Expected the sketchbook location to change but it did not.'
38+
);
39+
}
40+
}
41+
});
42+
const maybeUpdateCurrentSketchbookPath = async (
43+
sketchDirUri: URI | undefined = this.configService.tryGetSketchDirUri()
44+
) => {
45+
if (sketchDirUri) {
46+
const candidateSketchbookPath = await this.fileService.fsPath(
47+
sketchDirUri
48+
);
49+
if (candidateSketchbookPath !== this._currentSketchbookPath) {
50+
this._currentSketchbookPath = candidateSketchbookPath;
51+
}
52+
}
53+
};
54+
this.configService.onDidChangeSketchDirUri(
55+
maybeUpdateCurrentSketchbookPath
56+
);
57+
maybeUpdateCurrentSketchbookPath();
1558
}
1659

1760
override async onReady(): Promise<void> {
18-
this.update();
61+
this.windowService
62+
.isFirstWindow()
63+
// only the first window should warn about invalid sketch folder names.
64+
.then((firstWindow) => this.doUpdate(firstWindow));
1965
}
2066

2167
protected override update(): void {
22-
this.sketchService.getSketches({}).then((container) => {
23-
this.register(container);
24-
this.menuManager.update();
25-
});
68+
// on regular menu updates, do not raise warning dialogs about invalid sketch folder names
69+
this.doUpdate(false);
70+
}
71+
72+
private doUpdate(raiseInvalidSketchFoldersWarning?: boolean): void {
73+
this.sketchService
74+
.getSketches({})
75+
.then(({ container, sketchesInInvalidFolder }) => {
76+
if (
77+
raiseInvalidSketchFoldersWarning &&
78+
sketchesInInvalidFolder.length
79+
) {
80+
Promise.all(
81+
sketchesInInvalidFolder.map(async (ref) => {
82+
const fsPath = await this.fileService.fsPath(new URI(ref.uri));
83+
return {
84+
name: ref.name,
85+
path: fsPath,
86+
};
87+
})
88+
).then((dialogInputs) =>
89+
dialogInputs.reduce(async (acc, { name, path }) => {
90+
await acc;
91+
return remote.dialog.showMessageBox(remote.getCurrentWindow(), {
92+
title: nls.localize(
93+
'arduino/sketch/ignoringSketchWithBadNameTitle',
94+
'Ignoring sketch with bad name'
95+
),
96+
message: nls.localize(
97+
'arduino/sketch/ignoringSketchWithBadNameMessage',
98+
'The sketch "{0}" cannot be used. Sketch names must contain only basic letters and numbers. (ASCII-only with no spaces, and it cannot start with a number). To get rid of this message, remove the sketch from {1}',
99+
name,
100+
path
101+
),
102+
type: 'warning',
103+
}) as Promise<unknown>;
104+
}, Promise.resolve())
105+
);
106+
}
107+
this.register(container);
108+
this.menuManager.update();
109+
});
26110
}
27111

28112
override registerMenus(registry: MenuModelRegistry): void {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export interface VerifySketchParams {
2727
}
2828

2929
/**
30-
* - `"idle"` when neither verify, not upload is running,
30+
* - `"idle"` when neither verify, nor upload is running,
3131
* - `"explicit-verify"` when only verify is running triggered by the user, and
3232
* - `"automatic-verify"` is when the automatic verify phase is running as part of an upload triggered by the user.
3333
*/

Diff for: arduino-ide-extension/src/browser/dialogs/settings/settings.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ export class SettingsService {
112112
readonly onDidChange = this.onDidChangeEmitter.event;
113113
protected readonly onDidResetEmitter = new Emitter<Readonly<Settings>>();
114114
readonly onDidReset = this.onDidResetEmitter.event;
115+
protected readonly onWillChangeSoonEmitter = new Emitter<
116+
Readonly<Settings>
117+
>();
118+
readonly onWillChangeSoon = this.onWillChangeSoonEmitter.event;
115119

116120
protected ready = new Deferred<void>();
117121
protected _settings: Settings;
@@ -304,6 +308,7 @@ export class SettingsService {
304308
(config as any).sketchDirUri = sketchDirUri;
305309
(config as any).network = network;
306310
(config as any).locale = currentLanguage;
311+
this.onWillChangeSoonEmitter.fire(this._settings);
307312

308313
await Promise.all([
309314
this.savePreference('editor.fontSize', editorFontSize),
@@ -319,7 +324,6 @@ export class SettingsService {
319324
this.savePreference(SHOW_ALL_FILES_SETTING, sketchbookShowAllFiles),
320325
this.configService.setConfiguration(config),
321326
]);
322-
this.onDidChangeEmitter.fire(this._settings);
323327

324328
// after saving all the settings, if we need to change the language we need to perform a reload
325329
// Only reload if the language differs from the current locale. `nls.locale === undefined` signals english as well
@@ -335,6 +339,7 @@ export class SettingsService {
335339
this.commandService.executeCommand(ElectronCommands.RELOAD.id);
336340
}
337341

342+
this.onDidChangeEmitter.fire(this._settings);
338343
return true;
339344
}
340345
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export class SketchesServiceClientImpl
9090
if (!sketchDirUri) {
9191
return;
9292
}
93-
const container = await this.sketchService.getSketches({
93+
const { container } = await this.sketchService.getSketches({
9494
uri: sketchDirUri.toString(),
9595
});
9696
for (const sketch of SketchContainer.toArray(container)) {

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

+53-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ApplicationError } from '@theia/core/lib/common/application-error';
2+
import { nls } from '@theia/core/lib/common/nls';
23
import URI from '@theia/core/lib/common/uri';
34

45
export namespace SketchesError {
@@ -31,9 +32,14 @@ export const SketchesService = Symbol('SketchesService');
3132
export interface SketchesService {
3233
/**
3334
* Resolves to a sketch container representing the hierarchical structure of the sketches.
34-
* If `uri` is not given, `directories.user` will be user instead.
35+
* If `uri` is not given, `directories.user` will be user instead. The `sketchesInInvalidFolder`
36+
* array might contain sketches that were discovered, but due to their invalid name they were removed
37+
* from the `container`.
3538
*/
36-
getSketches({ uri }: { uri?: string }): Promise<SketchContainer>;
39+
getSketches({ uri }: { uri?: string }): Promise<{
40+
container: SketchContainer;
41+
sketchesInInvalidFolder: SketchRef[];
42+
}>;
3743

3844
/**
3945
* This is the TS implementation of `SketchLoad` from the CLI and should be replaced with a gRPC call eventually.
@@ -140,6 +146,51 @@ export interface Sketch extends SketchRef {
140146
readonly rootFolderFileUris: string[]; // `RootFolderFiles` (does not include the main sketch file)
141147
}
142148
export namespace Sketch {
149+
// (non-API) exported for the tests
150+
export const invalidSketchFolderNameMessage = nls.localize(
151+
'arduino/sketch/invalidSketchName',
152+
'Sketch names must start with a letter or number, followed by letters, numbers, dashes, dots and underscores. Maximum length is 63 characters.'
153+
);
154+
const invalidCloudSketchFolderNameMessage = nls.localize(
155+
'arduino/sketch/invalidCloudSketchName',
156+
'The name must consist of basic letters, numbers, or underscores. The maximum length is 36 characters.'
157+
);
158+
/**
159+
* `undefined` if the candidate sketch folder name is valid. Otherwise, the validation error message.
160+
* Based on the [specs](https://arduino.github.io/arduino-cli/latest/sketch-specification/#sketch-folders-and-files).
161+
*/
162+
export function validateSketchFolderName(
163+
candidate: string
164+
): string | undefined {
165+
return /^[0-9a-zA-Z]{1}[0-9a-zA-Z_\.-]{0,62}$/.test(candidate)
166+
? undefined
167+
: invalidSketchFolderNameMessage;
168+
}
169+
170+
/**
171+
* `undefined` if the candidate cloud sketch folder name is valid. Otherwise, the validation error message.
172+
* Based on how https://create.arduino.cc/editor/ works.
173+
*/
174+
export function validateCloudSketchFolderName(
175+
candidate: string
176+
): string | undefined {
177+
return /^[0-9a-zA-Z_]{1,36}$/.test(candidate)
178+
? undefined
179+
: invalidCloudSketchFolderNameMessage;
180+
}
181+
182+
/**
183+
* 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.
184+
* Throws an error if `candidate` is not valid.
185+
*/
186+
export function toValidCloudSketchFolderName(candidate: string): string {
187+
const errorMessage = validateSketchFolderName(candidate);
188+
if (errorMessage) {
189+
throw new Error(errorMessage);
190+
}
191+
return candidate.replace(/\./g, '_').replace(/-/g, '_').slice(0, 36);
192+
}
193+
143194
export function is(arg: unknown): arg is Sketch {
144195
if (!SketchRef.is(arg)) {
145196
return false;

0 commit comments

Comments
 (0)