diff --git a/e2e/newBranch.spec.ts b/e2e/newBranch.spec.ts new file mode 100644 index 000000000..f07067393 --- /dev/null +++ b/e2e/newBranch.spec.ts @@ -0,0 +1,70 @@ +/** + * Create New Branch: spaces typed or pasted into the name become separators on the fly, as in + * IDEA, so the name stays valid. Git is stubbed at the IPC layer. + */ +import type { Page } from "@playwright/test"; +import { expect, ipcCalls, stubDesktop, test } from "./fixtures"; + +const ROOT = "/dev/acme-api"; + +async function openNewBranchDialog(page: Page) { + await stubDesktop(page); + await page.evaluate(async (root) => { + const h = window.__ipc.handlers; + h.vcs_roots = () => [{ root, operation: null, head: "main", headCommit: "1111111" }]; + h.vcs_branch_info = () => ({ + head: "main", + headCommit: "1111111", + upstream: null, + ahead: 0, + behind: 0, + operation: null, + }); + h.vcs_status = () => []; + h.vcs_create_branch = () => null; + const vcsModule = "/src/features/vcs/store.ts"; + const { useVcs, refreshNow } = await import(/* @vite-ignore */ vcsModule); + useVcs.setState({ available: true, root, repo: root, branchesLoaded: false, stashesLoaded: false }); + await refreshNow(); + const actionsModule = "/src/platform/actions.ts"; + const { executeAction } = await import(/* @vite-ignore */ actionsModule); + void executeAction("vcs.newBranch"); + }, ROOT); + const dialog = page.getByRole("dialog", { name: "Create New Branch" }); + const input = dialog.getByRole("textbox", { name: "New branch name:" }); + await expect(input).toBeVisible(); + return { dialog, input }; +} + +test("a typed space becomes a separator and Create stays enabled", async ({ workbench: page }) => { + const { dialog, input } = await openNewBranchDialog(page); + await input.fill(""); + await input.pressSequentially("fix login redirect"); + await expect(input).toHaveValue("fix-login-redirect"); + await expect(dialog.getByText("Branch name contains invalid characters")).toHaveCount(0); + await expect(dialog.getByRole("button", { name: "Create", exact: true })).toBeEnabled(); + await dialog.getByRole("button", { name: "Create", exact: true }).click(); + const [call] = await ipcCalls(page, "vcs_create_branch"); + expect(call).toMatchObject({ name: "fix-login-redirect" }); +}); + +test("a space typed in the middle keeps the caret after it", async ({ workbench: page }) => { + const { input } = await openNewBranchDialog(page); + await input.fill("ab"); + await input.evaluate((el: HTMLInputElement) => el.setSelectionRange(1, 1)); + await page.keyboard.press("Space"); + await expect(input).toHaveValue("a-b"); + await page.keyboard.type("x"); + await expect(input).toHaveValue("a-xb"); +}); + +test("pasted text has its spaces replaced", async ({ workbench: page }) => { + const { dialog, input } = await openNewBranchDialog(page); + await input.fill(""); + await input.evaluate((el: HTMLInputElement) => { + el.focus(); + document.execCommand("insertText", false, "feature/add new thing"); + }); + await expect(input).toHaveValue("feature/add-new-thing"); + await expect(dialog.getByRole("button", { name: "Create", exact: true })).toBeEnabled(); +}); diff --git a/src/features/github/create/RemoteBranchDialog.test.tsx b/src/features/github/create/RemoteBranchDialog.test.tsx new file mode 100644 index 000000000..121d0908a --- /dev/null +++ b/src/features/github/create/RemoteBranchDialog.test.tsx @@ -0,0 +1,19 @@ +import { act, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { expect, it } from "vitest"; +import { GithubDialogHost } from "../ui/dialogs"; +import { remoteBranchNameDialog } from "./RemoteBranchDialog"; + +it("turns spaces typed in the remote branch name into separators", async () => { + const user = userEvent.setup(); + render(); + let result: Promise | undefined; + act(() => { + result = remoteBranchNameDialog(""); + }); + const input = await screen.findByRole("textbox", { name: "Remote branch name" }); + await user.type(input, "my feature"); + expect(input).toHaveValue("my-feature"); + await user.click(screen.getByRole("button", { name: "OK" })); + await expect(result).resolves.toBe("my-feature"); +}); diff --git a/src/features/github/create/RemoteBranchDialog.tsx b/src/features/github/create/RemoteBranchDialog.tsx index 9de05d63e..cd61dcb9c 100644 --- a/src/features/github/create/RemoteBranchDialog.tsx +++ b/src/features/github/create/RemoteBranchDialog.tsx @@ -1,6 +1,7 @@ /** "New Remote Branch for Pull Request": the name to push a local head without a push target to. */ import { useState } from "react"; -import { Dialog, Input, T } from "@/ui"; +import { cleanUpBranchNameOnTyping } from "@/platform/gitRefName"; +import { Dialog, Input, T, useSanitizedField } from "@/ui"; import { openDialog } from "../ui/dialogs"; import styles from "./Create.module.css"; @@ -11,6 +12,7 @@ export function remoteBranchNameDialog(suggested: string): Promise void }) { const [name, setName] = useState(props.suggested); + const { ref, clean } = useSanitizedField(cleanUpBranchNameOnTyping); const valid = !!name.trim(); return ( Remote branch name: - setName(e.target.value)} /> + setName(clean(e))} + />
Your changes will be pushed to the remote repository to create the pull request
diff --git a/src/features/vcs/__tests__/branchOps.test.ts b/src/features/vcs/__tests__/branchOps.test.ts index be0c1e78a..7b3e2db02 100644 --- a/src/features/vcs/__tests__/branchOps.test.ts +++ b/src/features/vcs/__tests__/branchOps.test.ts @@ -5,7 +5,7 @@ vi.mock("../dialogs", async (original) => ({ ...(await original()), promptText: mocks.prompt, })); -import { newBranch, newBranchInRepos } from "../branchOps"; +import { newBranch, newBranchInRepos, renameBranch } from "../branchOps"; import { useVcs } from "../store"; const branch = (head: string | null): BranchInfo => ({ @@ -61,3 +61,16 @@ it("prefills the current branch in the multi-repository dialog too", async () => await newBranchInRepos(["/p/front", "/p/back"]); expect(mocks.prompt.mock.calls[0]?.[0].initial).toBe("feature/ui"); }); + +it.each([ + ["New Branch", () => newBranch()], + ["New Branch from a revision", () => newBranch("origin/main")], + ["New Branch in several repositories", () => newBranchInRepos(["/p/front", "/p/back"])], + ["Rename Branch", () => renameBranch("main")], +])("%s turns a typed space into a separator, as IDEA does", async (_name, open) => { + await open(); + const { sanitize, validate } = mocks.prompt.mock.calls[0]![0]; + const typed: string = sanitize("fix login"); + expect(typed).toBe("fix-login"); + expect(validate(typed)).toBeNull(); +}); diff --git a/src/features/vcs/__tests__/promptDialog.test.tsx b/src/features/vcs/__tests__/promptDialog.test.tsx new file mode 100644 index 000000000..4a5e0ef92 --- /dev/null +++ b/src/features/vcs/__tests__/promptDialog.test.tsx @@ -0,0 +1,64 @@ +import { act, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanUpBranchNameOnTyping } from "@/platform/gitRefName"; +import { VcsDialogHost, promptText } from "../dialogs"; +import { validateBranchName } from "../branchOps"; + +const open = (initial = "") => + promptText({ + title: "Create New Branch", + label: "New branch name:", + initial, + okLabel: "Create", + sanitize: cleanUpBranchNameOnTyping, + validate: validateBranchName, + }); + +// The dialog stack is module state: close whatever a test left open. +afterEach(async () => { + for (const cancel of screen.queryAllByRole("button", { name: "Cancel" })) await userEvent.click(cancel); +}); + +describe("prompt dialog with a sanitizer (branch name)", () => { + it("replaces a typed space and keeps Create enabled", async () => { + const user = userEvent.setup(); + render(); + let result: ReturnType | undefined; + act(() => { + result = open(); + }); + const input = await screen.findByRole("textbox", { name: "New branch name:" }); + await user.type(input, "fix login redirect"); + expect(input).toHaveValue("fix-login-redirect"); + expect(screen.queryByText("Branch name contains invalid characters")).toBeNull(); + const create = screen.getByRole("button", { name: "Create" }); + expect(create).toBeEnabled(); + await user.click(create); + await expect(result).resolves.toEqual({ value: "fix-login-redirect", checked: false }); + }); + + it("converts spaces in pasted text", async () => { + const user = userEvent.setup(); + render(); + act(() => void open()); + const input = await screen.findByRole("textbox", { name: "New branch name:" }); + await user.click(input); + await user.paste("feature/add new thing"); + expect(input).toHaveValue("feature/add-new-thing"); + expect(screen.getByRole("button", { name: "Create" })).toBeEnabled(); + }); + + it("keeps the caret after the rewritten character when editing mid-text", async () => { + const user = userEvent.setup(); + render(); + act(() => void open("ab")); + const input = await screen.findByRole("textbox", { name: "New branch name:" }); + input.setSelectionRange(1, 1); + await user.keyboard(" "); + expect(input).toHaveValue("a-b"); + await waitFor(() => expect(input.selectionStart).toBe(2)); + await user.keyboard("x"); + expect(input).toHaveValue("a-xb"); + }); +}); diff --git a/src/features/vcs/branchOps.ts b/src/features/vcs/branchOps.ts index ab8c294c2..c0de39f38 100644 --- a/src/features/vcs/branchOps.ts +++ b/src/features/vcs/branchOps.ts @@ -8,6 +8,7 @@ import { ask } from "@/workbench/messages"; import { isFavoriteBranch, pushRecent, shortRemoteName } from "./branches"; import { showCommitRange } from "./CompareBranches"; import { confirmDialog, promptText } from "./dialogs"; +import { cleanUpBranchNameOnTyping } from "@/platform/gitRefName"; import { runOp, runRemote, saveAll } from "./ops"; import { conflictedFiles, currentRepo, refreshNow, repoName, useVcs } from "./store"; import { tx } from "@/platform/i18n"; @@ -160,6 +161,7 @@ export async function newBranch(from?: string) { initial: suggestion, mono: true, okLabel: "Create", + sanitize: cleanUpBranchNameOnTyping, validate: validateBranchName, validateInitial: true, checkbox: { label: "Checkout branch", initial: true }, @@ -236,6 +238,7 @@ export async function renameBranch(name: string) { initial: name, mono: true, okLabel: "Rename", + sanitize: cleanUpBranchNameOnTyping, validate: (v) => (v.trim() === name ? "Enter a new name" : validateBranchName(v)), }); if (!r) return; @@ -372,6 +375,7 @@ export async function newBranchInRepos(roots: readonly string[], from?: string) initial: from ? (from.includes("/") ? shortRemoteName(from) : "") : (useVcs.getState().branch?.head ?? ""), mono: true, okLabel: "Create", + sanitize: cleanUpBranchNameOnTyping, validate: validateBranchName, validateInitial: true, checkbox: { label: "Checkout branch", initial: true }, diff --git a/src/features/vcs/dialogs.tsx b/src/features/vcs/dialogs.tsx index 45a34a569..82bfefd18 100644 --- a/src/features/vcs/dialogs.tsx +++ b/src/features/vcs/dialogs.tsx @@ -1,7 +1,7 @@ /** Imperative dialogs for VCS actions, rendered by one overlay contribution. */ import { Fragment, useState, type ReactNode } from "react"; import { create } from "zustand"; -import { Checkbox, Dialog, Input, T } from "@/ui"; +import { Checkbox, Dialog, Input, T, useSanitizedField } from "@/ui"; import { confirm } from "@/workbench/messages"; import styles from "./common.module.css"; @@ -43,6 +43,8 @@ export interface PromptOptions { mono?: boolean; /** Returns an error message, or null when valid. */ validate?: (value: string) => string | null; + /** Rewrites the field as the user types or pastes (e.g. spaces in a branch name). */ + sanitize?: (value: string) => string; /** Show the prefilled value's problem (e.g. the branch exists) before it is edited. */ validateInitial?: boolean; /** Optional checkbox below the field. */ @@ -69,7 +71,8 @@ export function promptText(opts: PromptOptions): Promise { } function PromptDialog(props: PromptOptions & { onDone: (r: PromptResult | null) => void }) { - const { title, label, initial = "", placeholder, okLabel = "OK", mono, validate, checkbox, onDone } = props; + const { title, label, initial = "", placeholder, okLabel = "OK", mono, validate, sanitize, checkbox, onDone } = props; + const { ref, clean } = useSanitizedField(sanitize); const [value, setValue] = useState(initial); const [checked, setChecked] = useState(checkbox?.initial ?? false); const [touched, setTouched] = useState(!!props.validateInitial && initial !== ""); @@ -102,6 +105,7 @@ function PromptDialog(props: PromptOptions & { onDone: (r: PromptResult | null) )} { setTouched(true); - setValue(e.target.value); + setValue(clean(e)); }} /> {touched && error && value &&
{error}
} diff --git a/src/features/vcs/gitMenuDialogs.tsx b/src/features/vcs/gitMenuDialogs.tsx index 5fd455c17..5f0d36b04 100644 --- a/src/features/vcs/gitMenuDialogs.tsx +++ b/src/features/vcs/gitMenuDialogs.tsx @@ -5,9 +5,10 @@ import type { Branch, MergeOptions, Remote, Worktree } from "@/bindings"; import { executeAction } from "@/platform/actions"; import { call, commands } from "@/platform/ipc"; import { notifications } from "@/workbench/contributions"; -import { Checkbox, Dialog, Icon, IconButton, Input, Select, T, type SelectOption } from "@/ui"; +import { Checkbox, Dialog, Icon, IconButton, Input, Select, T, useSanitizedField, type SelectOption } from "@/ui"; import { sortLocal, sortRemote } from "./branches"; import { conflictActions, openConflictsInstead, validateBranchName } from "./branchOps"; +import { cleanUpBranchNameOnTyping } from "@/platform/gitRefName"; import { confirmDialog, openDialog } from "./dialogs"; import { errorMessage, runOp, runRemote, saveAll } from "./ops"; import { currentRepo, loadBranches, refreshNow, repoName, selectRepo, useVcs } from "./store"; @@ -687,6 +688,7 @@ function NewWorktreeDialog({ onClose }: { onClose: (created: boolean) => void }) const [source, setSource] = useState(null); const [createBranch, setCreateBranch] = useState(true); const [newBranch, setNewBranch] = useState(""); + const { ref: branchRef, clean: cleanBranch } = useSanitizedField(cleanUpBranchNameOnTyping); const [path, setPath] = useState(null); const [error, setError] = useState(null); const options = useMemo( @@ -744,11 +746,12 @@ function NewWorktreeDialog({ onClose }: { onClose: (created: boolean) => void }) {createBranch && ( setNewBranch(e.target.value)} + onChange={(e) => setNewBranch(cleanBranch(e))} /> )}
diff --git a/src/platform/gitRefName.test.ts b/src/platform/gitRefName.test.ts new file mode 100644 index 000000000..4fadbcce4 --- /dev/null +++ b/src/platform/gitRefName.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; +import { cleanUpBranchNameOnTyping } from "./gitRefName"; + +describe("cleanUpBranchNameOnTyping", () => { + it.each([ + // The reported case: a space typed or pasted between words. + ["feature x", "feature-x"], + ["fix login redirect", "fix-login-redirect"], + // Repeated spaces collapse to one separator. + ["a b", "a-b"], + ["feature ", "feature-"], + // Space pressed again after the substituted separator, and spaces around a typed one. + ["feature- ", "feature-"], + ["feature - x", "feature-x"], + ["a-b", "a-b"], + ["a--b", "a--b"], + // A leading space would form a leading `-`, which git rejects: dropped. + [" feature", "feature"], + [" ", ""], + // Tabs and other control characters count as whitespace too. + ["a\tb", "a-b"], + ["a\u007fb", "a-b"], + // Characters git forbids are dropped rather than substituted. + ["a~b^c:d?e*f[g\\h", "abcdefgh"], + ['say "hi"', "say-hi"], + ["a@{b", "ab"], + ["a..b", "a.b"], + ["a//b", "a/b"], + ["/a", "a"], + [".a", "a"], + ["-a", "a"], + ["a/.b", "a/b"], + ["a./b", "a/b"], + // Already valid names are untouched, including Unicode. + ["feature/login-v2", "feature/login-v2"], + ["release/1.2.3", "release/1.2.3"], + ["fix_bug", "fix_bug"], + ["功能/登录", "功能/登录"], + ["", ""], + ])("%j → %j", (input, expected) => { + expect(cleanUpBranchNameOnTyping(input)).toBe(expected); + }); + + it("is idempotent, so a cleaned field is stable under re-typing", () => { + for (const s of ["a b", " a", "a .. b", "@{ x", "x/ /y", "- - a"]) { + const once = cleanUpBranchNameOnTyping(s); + expect(cleanUpBranchNameOnTyping(once)).toBe(once); + } + }); +}); diff --git a/src/platform/gitRefName.ts b/src/platform/gitRefName.ts new file mode 100644 index 000000000..5802a54be --- /dev/null +++ b/src/platform/gitRefName.ts @@ -0,0 +1,32 @@ +/** Ports IntelliJ's `GitRefNameValidator.cleanUpBranchNameOnTyping` (git4idea). */ + +/** IntelliJ's advanced setting `git.branch.cleanup.symbol` (default `-`). */ +export const BRANCH_CLEANUP_SYMBOL = "-"; + +// Characters and sequences git forbids that cannot be repaired by substitution are dropped. +const DROP = /^\.|^-|^\/|[~:^?*"[\\]+|(?:@\{)+|\/(?=\/)|(?:\.(?=\.))+|\.(?=\/)/g; +// IntelliJ drops a dot after a slash with a lookbehind, which pre-16.4 WebKit cannot parse. +const DOT_AFTER_SLASH = /\/\./g; +// A run of whitespace / control characters becomes one separator. It also absorbs separators +// mixed into or next to it, so pressing Space again after the substituted one doesn't stack them (IntelliJ +// collapses only spaces typed in one go). +const SEPARATOR = BRANCH_CLEANUP_SYMBOL.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +const REPLACE = new RegExp(`(?:${SEPARATOR}|[ \\p{Cc}])*[ \\p{Cc}](?:${SEPARATOR}|[ \\p{Cc}])*`, "gu"); + +function cleanOnce(name: string): string { + return name.replace(DROP, "").replace(DOT_AFTER_SLASH, "/").replace(REPLACE, BRANCH_CLEANUP_SYMBOL); +} + +/** + * Makes `name` typable as a branch name: forbidden characters are dropped and spaces become the + * cleanup symbol, so a space never reaches the validator. Runs to a fixed point because the + * substitution can itself form a forbidden start (" x" → "-x" → "x"). + */ +export function cleanUpBranchNameOnTyping(name: string): string { + let cleaned = name; + for (;;) { + const next = cleanOnce(cleaned); + if (next === cleaned) return cleaned; + cleaned = next; + } +} diff --git a/src/ui/index.ts b/src/ui/index.ts index 5633d0a1d..8cab09729 100644 --- a/src/ui/index.ts +++ b/src/ui/index.ts @@ -36,4 +36,5 @@ export { lazyComponent, prefetchLazyComponents } from "./lazy"; export { T } from "./T"; export { selectableText, copyMenuFor, copyText } from "./selectableText"; export { useSelectionBoundary } from "./useSelectionBoundary"; +export { useSanitizedField } from "./useSanitizedField"; export * from "./rowSelection"; diff --git a/src/ui/useSanitizedField.ts b/src/ui/useSanitizedField.ts new file mode 100644 index 000000000..fd684608e --- /dev/null +++ b/src/ui/useSanitizedField.ts @@ -0,0 +1,34 @@ +import { useLayoutEffect, useRef, type ChangeEvent } from "react"; + +/** + * Rewrites what the user types or pastes into a text input as IntelliJ does for ref names, + * keeping the caret where the typed character was. Use as + * ` setValue(clean(e))} />`. + */ +export function useSanitizedField(sanitize?: (value: string) => string) { + const ref = useRef(null); + const caret = useRef(null); + + const place = () => { + if (caret.current === null) return; + ref.current?.setSelectionRange(caret.current, caret.current); + caret.current = null; + }; + // A controlled input whose text was rewritten loses its caret on re-render. + useLayoutEffect(place); + + const clean = (e: ChangeEvent): string => { + const raw = e.target.value; + const value = sanitize ? sanitize(raw) : raw; + if (value !== raw) { + const at = e.target.selectionStart ?? raw.length; + caret.current = sanitize ? sanitize(raw.slice(0, at)).length : at; + // Nothing re-renders when the rewrite equals the current state (a dropped character), and + // React then puts the caret at the end once this handler returns: a microtask runs after that. + queueMicrotask(place); + } + return value; + }; + + return { ref, clean }; +}