diff --git a/e2e/playwright/code-pane-and-errors.spec.ts b/e2e/playwright/code-pane-and-errors.spec.ts index 0e8706c8ec..7f2ffc95c8 100644 --- a/e2e/playwright/code-pane-and-errors.spec.ts +++ b/e2e/playwright/code-pane-and-errors.spec.ts @@ -280,7 +280,7 @@ test( await expect(page.getByRole('link', { name: 'bracket' })).toBeVisible() await expect(page.getByText('router-template-slate')).toBeVisible() - await expect(page.getByText('New Project')).toBeVisible() + await expect(page.getByText('Create project')).toBeVisible() }) await test.step('Opening the router-template project should load', async () => { diff --git a/e2e/playwright/fixtures/cmdBarFixture.ts b/e2e/playwright/fixtures/cmdBarFixture.ts index 65d1fafda4..58a6e88a8a 100644 --- a/e2e/playwright/fixtures/cmdBarFixture.ts +++ b/e2e/playwright/fixtures/cmdBarFixture.ts @@ -142,4 +142,20 @@ export class CmdBarFixture { await promptEditCommand.first().click() } } + + get cmdSearchInput() { + return this.page.getByTestId('cmd-bar-search') + } + + get argumentInput() { + return this.page.getByTestId('cmd-bar-arg-value') + } + + get cmdOptions() { + return this.page.getByTestId('cmd-bar-option') + } + + chooseCommand = async (commandName: string) => { + await this.cmdOptions.getByText(commandName).click() + } } diff --git a/e2e/playwright/fixtures/toolbarFixture.ts b/e2e/playwright/fixtures/toolbarFixture.ts index 689d125565..56a1794b02 100644 --- a/e2e/playwright/fixtures/toolbarFixture.ts +++ b/e2e/playwright/fixtures/toolbarFixture.ts @@ -63,6 +63,10 @@ export class ToolbarFixture { this.exeIndicator = page.getByTestId('model-state-indicator-execution-done') } + get logoLink() { + return this.page.getByTestId('app-logo') + } + startSketchPlaneSelection = async () => doAndWaitForImageDiff(this.page, () => this.startSketchBtn.click(), 500) diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts index 38b8324732..4ad54b025d 100644 --- a/e2e/playwright/projects.spec.ts +++ b/e2e/playwright/projects.spec.ts @@ -172,7 +172,7 @@ test( await expect(page.getByRole('link', { name: 'bracket' })).toBeVisible() await expect(page.getByText('broken-code')).toBeVisible() await expect(page.getByText('bracket')).toBeVisible() - await expect(page.getByText('New Project')).toBeVisible() + await expect(page.getByText('Create project')).toBeVisible() }) await test.step('opening broken code project should clear the scene and show the error', async () => { // Go back home. @@ -253,7 +253,7 @@ test( await expect(page.getByRole('link', { name: 'bracket' })).toBeVisible() await expect(page.getByText('empty')).toBeVisible() await expect(page.getByText('bracket')).toBeVisible() - await expect(page.getByText('New Project')).toBeVisible() + await expect(page.getByText('Create project')).toBeVisible() }) await test.step('opening empty code project should clear the scene', async () => { // Go back home. @@ -985,6 +985,107 @@ test.describe(`Project management commands`, () => { }) } ) + test(`Create a new project with a colliding name`, async ({ + context, + homePage, + toolbar, + cmdBar, + }) => { + const projectName = 'test-project' + await test.step(`Setup`, async () => { + await context.folderSetupFn(async (dir) => { + const projectDir = path.join(dir, projectName) + await Promise.all([fsp.mkdir(projectDir, { recursive: true })]) + await Promise.all([ + fsp.copyFile( + executorInputPath('router-template-slate.kcl'), + path.join(projectDir, 'main.kcl') + ), + ]) + }) + await homePage.expectState({ + projectCards: [ + { + title: projectName, + fileCount: 1, + }, + ], + sortBy: 'last-modified-desc', + }) + }) + + await test.step('Create a new project with the same name', async () => { + await cmdBar.openCmdBar() + await cmdBar.chooseCommand('create project') + await cmdBar.expectState({ + stage: 'arguments', + commandName: 'Create project', + currentArgKey: 'name', + currentArgValue: '', + headerArguments: { + Name: '', + }, + highlightedHeaderArg: 'name', + }) + await cmdBar.argumentInput.fill(projectName) + await cmdBar.progressCmdBar() + }) + + await test.step(`Check the project was created with a non-colliding name`, async () => { + await toolbar.logoLink.click() + await homePage.expectState({ + projectCards: [ + { + title: projectName + '-1', + fileCount: 1, + }, + { + title: projectName, + fileCount: 1, + }, + ], + sortBy: 'last-modified-desc', + }) + }) + + await test.step('Create another project with the same name', async () => { + await cmdBar.openCmdBar() + await cmdBar.chooseCommand('create project') + await cmdBar.expectState({ + stage: 'arguments', + commandName: 'Create project', + currentArgKey: 'name', + currentArgValue: '', + headerArguments: { + Name: '', + }, + highlightedHeaderArg: 'name', + }) + await cmdBar.argumentInput.fill(projectName) + await cmdBar.progressCmdBar() + }) + + await test.step(`Check the second project was created with a non-colliding name`, async () => { + await toolbar.logoLink.click() + await homePage.expectState({ + projectCards: [ + { + title: projectName + '-2', + fileCount: 1, + }, + { + title: projectName + '-1', + fileCount: 1, + }, + { + title: projectName, + fileCount: 1, + }, + ], + sortBy: 'last-modified-desc', + }) + }) + }) }) test( @@ -1391,7 +1492,7 @@ extrude001 = extrude(200, sketch001)`) await page.getByTestId('app-logo').click() await expect( - page.getByRole('button', { name: 'New project' }) + page.getByRole('button', { name: 'Create project' }) ).toBeVisible() for (let i = 1; i <= 10; i++) { @@ -1465,7 +1566,7 @@ test( await expect(page.getByRole('link', { name: 'bracket' })).toBeVisible() await expect(page.getByText('router-template-slate')).toBeVisible() - await expect(page.getByText('New Project')).toBeVisible() + await expect(page.getByText('Create project')).toBeVisible() }) await test.step('Opening the router-template project should load the stream', async () => { @@ -1494,7 +1595,7 @@ test( await expect(page.getByRole('link', { name: 'bracket' })).toBeVisible() await expect(page.getByText('router-template-slate')).toBeVisible() - await expect(page.getByText('New Project')).toBeVisible() + await expect(page.getByText('Create project')).toBeVisible() }) } ) diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 01164207b4..f91f0dbc77 100644 --- a/e2e/playwright/test-utils.ts +++ b/e2e/playwright/test-utils.ts @@ -1078,7 +1078,7 @@ export async function createProject({ returnHome?: boolean }) { await test.step(`Create project and navigate to it`, async () => { - await page.getByRole('button', { name: 'New project' }).click() + await page.getByRole('button', { name: 'Create project' }).click() await page.getByRole('textbox', { name: 'Name' }).fill(name) await page.getByRole('button', { name: 'Continue' }).click() diff --git a/src/components/CommandBar/CommandArgOptionInput.tsx b/src/components/CommandBar/CommandArgOptionInput.tsx index 0391ae4798..57ae4ac3e5 100644 --- a/src/components/CommandBar/CommandArgOptionInput.tsx +++ b/src/components/CommandBar/CommandArgOptionInput.tsx @@ -134,6 +134,7 @@ function CommandArgOptionInput({ !event.target.disabled && setQuery(event.target.value) diff --git a/src/components/CommandComboBox.tsx b/src/components/CommandComboBox.tsx index b71ad36a5a..5e3fbe3735 100644 --- a/src/components/CommandComboBox.tsx +++ b/src/components/CommandComboBox.tsx @@ -52,6 +52,7 @@ function CommandComboBox({ className="w-5 h-5 bg-primary/10 dark:bg-primary text-primary dark:text-inherit" /> setQuery(event.target.value)} className="w-full bg-transparent focus:outline-none selection:bg-primary/20 dark:selection:bg-primary/40 dark:focus:outline-none" onKeyDown={(event) => { @@ -85,6 +86,7 @@ function CommandComboBox({ value={option} className="flex items-center gap-4 px-4 py-1.5 first:mt-2 last:mb-2 ui-active:bg-primary/10 dark:ui-active:bg-chalkboard-90 ui-disabled:!text-chalkboard-50" disabled={optionIsDisabled(option)} + data-testid={`cmd-bar-option`} > {'icon' in option && option.icon && ( diff --git a/src/components/ProjectsContextProvider.tsx b/src/components/ProjectsContextProvider.tsx index 21f672f88d..ba71242a2d 100644 --- a/src/components/ProjectsContextProvider.tsx +++ b/src/components/ProjectsContextProvider.tsx @@ -18,6 +18,7 @@ import { getNextProjectIndex, interpolateProjectNameWithIndex, doesProjectNameNeedInterpolated, + getUniqueProjectName, } from 'lib/desktopFS' import { useSettingsAuthContext } from 'hooks/useSettingsAuthContext' import useStateMachineCommands from 'hooks/useStateMachineCommands' @@ -195,15 +196,11 @@ const ProjectsContextDesktop = ({ : settings.projects.defaultProjectName.current ).trim() - if (doesProjectNameNeedInterpolated(name)) { - const nextIndex = getNextProjectIndex(name, input.projects) - name = interpolateProjectNameWithIndex(name, nextIndex) - } - - await createNewProjectDirectory(name) + const uniqueName = getUniqueProjectName(name, input.projects) + await createNewProjectDirectory(uniqueName) return { - message: `Successfully created "${name}"`, + message: `Successfully created "${uniqueName}"`, name, } }), diff --git a/src/lib/desktopFS.test.ts b/src/lib/desktopFS.test.ts new file mode 100644 index 0000000000..a590c64f5b --- /dev/null +++ b/src/lib/desktopFS.test.ts @@ -0,0 +1,58 @@ +import { getUniqueProjectName } from './desktopFS' +import { FileEntry } from './project' + +/** Create a dummy project */ +function project(name: string, children?: FileEntry[]): FileEntry { + return { + name, + children: children || [ + { name: 'main.kcl', children: null, path: 'main.kcl' }, + ], + path: `/projects/${name}`, + } +} + +describe(`Getting unique project names`, () => { + it(`should return the same name if no conflicts`, () => { + const projectName = 'new-project' + const projects = [project('existing-project'), project('another-project')] + const result = getUniqueProjectName(projectName, projects) + expect(result).toBe(projectName) + }) + it(`should return a unique name if there is a conflict`, () => { + const projectName = 'existing-project' + const projects = [project('existing-project'), project('another-project')] + const result = getUniqueProjectName(projectName, projects) + expect(result).toBe('existing-project-1') + }) + it(`should increment an ending index until a unique one is found`, () => { + const projectName = 'existing-project-1' + const projects = [ + project('existing-project'), + project('existing-project-1'), + project('existing-project-2'), + ] + const result = getUniqueProjectName(projectName, projects) + expect(result).toBe('existing-project-3') + }) + it(`should prefer the formatting of the index identifier if present`, () => { + const projectName = 'existing-project-$nn' + const projects = [ + project('existing-project'), + project('existing-project-1'), + project('existing-project-2'), + ] + const result = getUniqueProjectName(projectName, projects) + expect(result).toBe('existing-project-03') + }) + it(`be able to get an incrementing index regardless of padding zeroes`, () => { + const projectName = 'existing-project-$nn' + const projects = [ + project('existing-project'), + project('existing-project-01'), + project('existing-project-2'), + ] + const result = getUniqueProjectName(projectName, projects) + expect(result).toBe('existing-project-03') + }) +}) diff --git a/src/lib/desktopFS.ts b/src/lib/desktopFS.ts index c09845127a..e7b9bf1df4 100644 --- a/src/lib/desktopFS.ts +++ b/src/lib/desktopFS.ts @@ -54,8 +54,10 @@ export function getNextProjectIndex( const matches = projects.map((project) => project.name?.match(regex)) const indices = matches .filter(Boolean) - .map((match) => match![1]) - .map(Number) + .map((match) => (match !== null ? match[1] : '-1')) + .map((maybeMatchIndex) => { + return parseInt(maybeMatchIndex || '0', 10) + }) const maxIndex = Math.max(...indices, -1) return maxIndex + 1 } @@ -83,6 +85,33 @@ export function doesProjectNameNeedInterpolated(projectName: string) { return projectName.includes(INDEX_IDENTIFIER) } +/** + * Given a target name, which may include our magic index interpolation string, + * and a list of projects, return a unique name that doesn't conflict with any + * of the existing projects, incrementing any ending number if necessary. + * @param name + * @param projects + * @returns + */ +export function getUniqueProjectName(name: string, projects: FileEntry[]) { + // The name may have our magic index interpolation string in it + const needsInterpolation = doesProjectNameNeedInterpolated(name) + + if (needsInterpolation) { + const nextIndex = getNextProjectIndex(name, projects) + return interpolateProjectNameWithIndex(name, nextIndex) + } else { + let newName = name + while (projects.some((project) => project.name === newName)) { + const nameEndsWithNumber = newName.match(/\d+$/) + newName = nameEndsWithNumber + ? newName.replace(/\d+$/, (num) => `${parseInt(num, 10) + 1}`) + : `${name}-1` + } + return newName + } +} + function escapeRegExpChars(string: string) { return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } diff --git a/src/routes/Home.tsx b/src/routes/Home.tsx index 3c48396379..eb14b69c1b 100644 --- a/src/routes/Home.tsx +++ b/src/routes/Home.tsx @@ -148,7 +148,7 @@ const Home = () => { }} data-testid="home-new-file" > - New project + Create project