Skip to content

Commit e4d12be

Browse files
author
Akos Kitta
committed
fix: do not prompt invalid sketch folder names
Signed-off-by: Akos Kitta <[email protected]>
1 parent 21ac9e0 commit e4d12be

File tree

3 files changed

+114
-55
lines changed

3 files changed

+114
-55
lines changed

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { WindowService } from '@theia/core/lib/browser/window/window-service';
77
import { nls } from '@theia/core/lib/common/nls';
88
import { inject, injectable } from '@theia/core/shared/inversify';
99
import { WorkspaceInput } from '@theia/workspace/lib/browser/workspace-service';
10-
import * as dateFormat from 'dateformat';
1110
import { CurrentSketch } from '../sketches-service-client-impl';
1211
import { StartupTask } from '../../electron-common/startup-task';
1312
import { ArduinoMenus } from '../menu/arduino-menus';
@@ -143,10 +142,9 @@ export class SaveAsSketch extends CloudSketchContribution {
143142

144143
// If target does not exist, propose a `directories.user`/${sketch.name} path
145144
// If target exists, propose `directories.user`/${sketch.name}_copy_${yyyymmddHHMMss}
145+
// IDE2 must never prompt an invalid sketch folder name (https://github.com/arduino/arduino-ide/pull/1833#issuecomment-1412569252)
146146
const defaultUri = containerDirUri.resolve(
147-
exists
148-
? `${sketch.name}_copy_${dateFormat(new Date(), 'yyyymmddHHMMss')}`
149-
: sketch.name
147+
Sketch.toValidSketchFolderName(sketch.name, exists)
150148
);
151149
const defaultPath = await this.fileService.fsPath(defaultUri);
152150
return await this.promptLocalSketchFolderDestination(defaultPath);

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

+42-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ApplicationError } from '@theia/core/lib/common/application-error';
22
import { nls } from '@theia/core/lib/common/nls';
3+
import * as dateFormat from 'dateformat';
34
import URI from '@theia/core/lib/common/uri';
45

56
export namespace SketchesError {
@@ -152,6 +153,12 @@ export interface Sketch extends SketchRef {
152153
readonly rootFolderFileUris: string[]; // `RootFolderFiles` (does not include the main sketch file)
153154
}
154155
export namespace Sketch {
156+
// (non-API) exported for the tests
157+
export const defaultSketchFolderName = 'sketch';
158+
// (non-API) exported for the tests
159+
export const defaultFallbackFirstChar = '0';
160+
// (non-API) exported for the tests
161+
export const defaultFallbackChar = '_';
155162
// (non-API) exported for the tests
156163
export const invalidSketchFolderNameMessage = nls.localize(
157164
'arduino/sketch/invalidSketchName',
@@ -186,15 +193,43 @@ export namespace Sketch {
186193
}
187194

188195
/**
189-
* 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.
190-
* Throws an error if `candidate` is not valid.
196+
* Transforms the `candidate` argument into a valid sketch folder name by replacing all invalid characters with underscore (`_`) and trimming the string after 63 characters.
197+
* If the argument is falsy, returns with `"sketch"`.
191198
*/
192-
export function toValidCloudSketchFolderName(candidate: string): string {
193-
const errorMessage = validateSketchFolderName(candidate);
194-
if (errorMessage) {
195-
throw new Error(errorMessage);
199+
export function toValidSketchFolderName(
200+
candidate: string,
201+
appendTimestampSuffix: boolean | Date = false
202+
): string {
203+
const validName = candidate
204+
? candidate
205+
.replace(/^[^0-9a-zA-Z]{1}/g, defaultFallbackFirstChar)
206+
.replace(/[^0-9a-zA-Z_\.-]/g, defaultFallbackChar)
207+
.slice(0, 63)
208+
: defaultSketchFolderName;
209+
if (appendTimestampSuffix) {
210+
return `${validName.slice(0, 63 - timestampSuffixLength)}${
211+
typeof appendTimestampSuffix === 'boolean'
212+
? timestampSuffix()
213+
: timestampSuffix(appendTimestampSuffix)
214+
}`;
196215
}
197-
return candidate.replace(/\./g, '_').replace(/-/g, '_').slice(0, 36);
216+
return validName;
217+
}
218+
219+
const copy = '_copy_';
220+
const datetimeFormat = 'yyyymmddHHMMss';
221+
const timestampSuffixLength = copy.length + datetimeFormat.length;
222+
function timestampSuffix(now = new Date()): string {
223+
return `${copy}${dateFormat(now, datetimeFormat)}`;
224+
}
225+
226+
/**
227+
* Transforms the `candidate` argument into a valid cloud sketch folder name by replacing all invalid characters with underscore and trimming the string after 36 characters.
228+
*/
229+
export function toValidCloudSketchFolderName(candidate: string): string {
230+
return candidate
231+
? candidate.replace(/[^0-9a-zA-Z_]/g, defaultFallbackChar).slice(0, 36)
232+
: defaultSketchFolderName;
198233
}
199234

200235
export function is(arg: unknown): arg is Sketch {

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

+70-44
Original file line numberDiff line numberDiff line change
@@ -69,51 +69,77 @@ describe('sketch', () => {
6969
});
7070
});
7171

72+
describe('toValidSketchFolderName', () => {
73+
[
74+
['', Sketch.defaultSketchFolderName],
75+
[' ', Sketch.defaultFallbackFirstChar],
76+
[' ', Sketch.defaultFallbackFirstChar + Sketch.defaultFallbackChar],
77+
[
78+
'0123456789012345678901234567890123456789012345678901234567890123',
79+
'012345678901234567890123456789012345678901234567890123456789012',
80+
],
81+
['foo bar', 'foo_bar'],
82+
['vAlid', 'vAlid'],
83+
].map(([input, expected]) =>
84+
toMapIt(input, expected, Sketch.toValidSketchFolderName)
85+
);
86+
});
87+
88+
describe('toValidSketchFolderName with timestamp suffix', () => {
89+
const epoch = new Date(0);
90+
const epochSuffix = '_copy_19700101010000';
91+
[
92+
['', Sketch.defaultSketchFolderName + epochSuffix],
93+
[' ', Sketch.defaultFallbackFirstChar + epochSuffix],
94+
[
95+
' ',
96+
Sketch.defaultFallbackFirstChar +
97+
Sketch.defaultFallbackChar +
98+
epochSuffix,
99+
],
100+
[
101+
'0123456789012345678901234567890123456789012345678901234567890123',
102+
'0123456789012345678901234567890123456789012' + epochSuffix,
103+
],
104+
['foo bar', 'foo_bar' + epochSuffix],
105+
['vAlid', 'vAlid' + epochSuffix],
106+
].map(([input, expected]) =>
107+
toMapIt(input, expected, (input: string) =>
108+
Sketch.toValidSketchFolderName(input, epoch)
109+
)
110+
);
111+
});
112+
72113
describe('toValidCloudSketchFolderName', () => {
73-
(
114+
[
115+
['sketch', 'sketch'],
116+
['can-contain-slash-and-dot.ino', 'can_contain_slash_and_dot_ino'],
117+
['regex++', 'regex__'],
118+
['dots...', 'dots___'],
119+
['No Spaces', 'No_Spaces'],
120+
['_startsWithUnderscore', '_startsWithUnderscore'],
121+
['Invalid+Char.ino', 'Invalid_Char_ino'],
122+
['', 'sketch'],
123+
['/', '_'],
124+
['//trash/', '__trash_'],
74125
[
75-
['sketch', 'sketch'],
76-
['can-contain-slash-and-dot.ino', 'can_contain_slash_and_dot_ino'],
77-
['regex++'],
78-
['dots...', 'dots___'],
79-
['No Spaces'],
80-
['_invalidToStartWithUnderscore'],
81-
['Invalid+Char.ino'],
82-
[''],
83-
['/'],
84-
['//trash/'],
85-
[
86-
'63Length_012345678901234567890123456789012345678901234567890123',
87-
'63Length_012345678901234567890123456',
88-
],
89-
['TooLong__0123456789012345678901234567890123456789012345678901234'],
90-
] as [string, string?][]
91-
).map(([input, expected]) => {
92-
it(`'${input}' should ${expected ? '' : 'not '}map the ${
93-
!expected ? 'invalid ' : ''
94-
}sketch folder name to a valid cloud sketch folder name${
95-
expected ? `: '${expected}'` : ''
96-
}`, () => {
97-
if (!expected) {
98-
try {
99-
Sketch.toValidCloudSketchFolderName(input);
100-
throw new Error(
101-
`Expected an error when mapping ${input} to a valid sketch folder name.`
102-
);
103-
} catch (err) {
104-
if (err instanceof Error) {
105-
expect(err.message).to.be.equal(
106-
Sketch.invalidSketchFolderNameMessage
107-
);
108-
} else {
109-
throw err;
110-
}
111-
}
112-
} else {
113-
const actual = Sketch.toValidCloudSketchFolderName(input);
114-
expect(actual).to.be.equal(expected);
115-
}
116-
});
117-
});
126+
'63Length_012345678901234567890123456789012345678901234567890123',
127+
'63Length_012345678901234567890123456',
128+
],
129+
].map(([input, expected]) =>
130+
toMapIt(input, expected, Sketch.toValidCloudSketchFolderName, true)
131+
);
118132
});
119133
});
134+
135+
function toMapIt(
136+
input: string,
137+
expected: string,
138+
testMe: (input: string) => string,
139+
cloud = false
140+
): Mocha.Test {
141+
return it(`should map the '${input}' ${
142+
cloud ? 'cloud ' : ''
143+
}sketch folder name to '${expected}'`, () =>
144+
expect(testMe(input)).to.be.equal(expected));
145+
}

0 commit comments

Comments
 (0)