From 9189d0a5fbfac0d181a8ac5ee2873139f9208a84 Mon Sep 17 00:00:00 2001 From: CX330 Date: Thu, 17 Sep 2026 18:54:06 +0800 Subject: [PATCH] Add auto indentation to the code editor Reduce manual formatting during interviews by preserving the current line indentation and adding one level after opening delimiters and Python block statements. Keep delimiter pairs readable by moving matching closing delimiters onto a separate line when appropriate. Avoid treating comment-only lines as block openers so comments do not introduce unexpected indentation. Add regression tests to cover indentation behavior, comments, and C++ preprocessor directives. --- scripts/browser-check.cjs | 50 ++++++++++++ tests/browser/editor.test.js | 150 ++++++++++++++++++++++++++++++++++- web/editor.js | 55 +++++++++++++ web/interview.js | 7 +- 4 files changed, 260 insertions(+), 2 deletions(-) diff --git a/scripts/browser-check.cjs b/scripts/browser-check.cjs index 2c64b6df..b2f78a12 100644 --- a/scripts/browser-check.cjs +++ b/scripts/browser-check.cjs @@ -8,6 +8,7 @@ // cares about. Those wait for the thing being tested; `networkidle` waited for // the whole page and cost the offline flow about fifty seconds of it. const { chromium } = require(process.env.PLAYWRIGHT_PATH); +const assert = require("node:assert/strict"); const fs = require("fs"); const http = require("http"); const { spawn } = require("child_process"); @@ -33,6 +34,54 @@ function interviewUrl(problemId) { return `${process.env.BASE_URL}/interview?problem=${scenario(problemId).page}&duration=20`; } +async function checkEditorNewlines(page) { + const editor = page.getByLabel("Code editor"); + const cases = [ + { name: "existing indentation", language: "JavaScript", value: "\t call();", expected: "\t call();\n\t " }, + { name: "matching delimiters", language: "JavaScript", value: " {}", start: 5, expected: " {\n \n }", caret: 14 }, + { name: "selection replacement", language: "JavaScript", value: " beforeREMOVEafter", start: 10, end: 16, expected: " before\n after", caret: 15 }, + { name: "Python block", language: "Python 3", value: " if ready:", expected: " if ready:\n " }, + { name: "Python comment", language: "Python 3", value: " # Steps:", expected: " # Steps:\n " }, + { name: "non-Python colon", language: "JavaScript", value: " case 1:", expected: " case 1:\n " }, + { name: "line comment", language: "JavaScript", value: " // setup {", expected: " // setup {\n " }, + { name: "block comment", language: "C++", value: " /* setup {", expected: " /* setup {\n " }, + { name: "preprocessor directive", language: "C++", value: " #define BLOCK {", expected: " #define BLOCK {\n " }, + { name: "trailing line comment", language: "JavaScript", value: " work(); // {", expected: " work(); // {\n " }, + { name: "trailing Python comment", language: "Python 3", value: " value = 1 # note:", expected: " value = 1 # note:\n " }, + { name: "Python opener before comment", language: "Python 3", value: " if ready: # note", expected: " if ready: # note\n " }, + { name: "brace before comment", language: "JavaScript", value: " if (ready) { // note", expected: " if (ready) { // note\n " }, + { name: "code after block comment", language: "C++", value: " /* seed */ if (ready) {", expected: " /* seed */ if (ready) {\n " }, + { name: "trailing block comment", language: "C++", value: " if (ready) { /* note */", expected: " if (ready) { /* note */\n " }, + { name: "quoted comment marker", language: "JavaScript", value: ' if (url === "https://example.com") { // note', expected: ' if (url === "https://example.com") { // note\n ' }, + { name: "quoted Python comment marker", language: "Python 3", value: " if tag == '#': # note", expected: " if tag == '#': # note\n " }, + { name: "escaped quote before comment marker", language: "Python 3", value: String.raw` if tag == 'can\'t #': # note`, expected: String.raw` if tag == 'can\'t #': # note` + "\n " }, + { name: "delimiters in trailing comment", language: "JavaScript", value: " work(); // {}", start: 16, expected: " work(); // {\n }", caret: 21 }, + ]; + for (const { name, language, value, start = value.length, end = start, expected, caret = expected.length } of cases) { + await page.getByRole("button", { name: language, exact: true }).click(); + await editor.fill(value); + await editor.evaluate((node, range) => node.setSelectionRange(...range), [start, end]); + await editor.press("Enter"); + assert.deepEqual(await editor.evaluate((node) => ({ + value: node.value, start: node.selectionStart, end: node.selectionEnd, + })), { value: expected, start: caret, end: caret }, name); + assert.equal(await page.locator("#editor-highlight code").textContent(), expected, `${name}: highlight`); + assert.equal(await page.locator("#editor-lines").textContent(), + expected.split("\n").map((_, index) => index + 1).join("\n"), `${name}: line numbers`); + + await editor.press("ControlOrMeta+z"); + assert.equal(await editor.inputValue(), value, `${name}: undo`); + await editor.press("ControlOrMeta+Shift+z"); + assert.equal(await editor.inputValue(), expected, `${name}: redo`); + + const otherLanguage = language === "Python 3" ? "JavaScript" : "Python 3"; + await page.getByRole("button", { name: otherLanguage, exact: true }).click(); + await page.getByRole("button", { name: language, exact: true }).click(); + assert.equal(await editor.inputValue(), expected, `${name}: retained after switching languages`); + } + console.log(`editor: ${cases.length} Enter cases passed, including undo, redo and language switching`); +} + const soakSeconds = Number(process.env.BROWSER_CHECK_SOAK_SECONDS || "0"); if (!Number.isSafeInteger(soakSeconds) || soakSeconds < 0) { throw new Error("BROWSER_CHECK_SOAK_SECONDS must be a whole number of seconds"); @@ -533,6 +582,7 @@ async function isolateRustAgent(roomName, rustAgentIdentity, timeoutMs = 120000) await clearMediaGate(page); await page.getByRole("heading", { name: scenarioTitle("two-sum"), level: 1 }).waitFor(); await page.getByText("Offline", { exact: true }).waitFor(); + await checkEditorNewlines(page); // The `""` branch that used to be here is gone. It set the global from an // init script and expected "not wired up yet", which cannot work: the // page also loads /runtime-config.js, which assigns the same global, so diff --git a/tests/browser/editor.test.js b/tests/browser/editor.test.js index 29684acc..a4fe733e 100644 --- a/tests/browser/editor.test.js +++ b/tests/browser/editor.test.js @@ -1,7 +1,155 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { indentSelection } from "../../web/editor.js"; +import { indentNewline, indentSelection } from "../../web/editor.js"; + +test("Enter preserves the current line's spaces and tabs", () => { + for (const indentation of ["", " ", " ", "\t", "\t "]) { + const value = `previous\n${indentation}call();`; + const expected = `${value}\n${indentation}`; + assert.deepEqual( + indentNewline(value, value.length, value.length, "javascript"), + { value: expected, start: expected.length, end: expected.length } + ); + } +}); + +test("Enter adds a level after opening delimiters and Python colons", () => { + for (const [value, language] of [ + [" if (ready) {", "javascript"], + [" items = [", "python"], + [" call(", "java"], + [" if ready:", "python"], + [" if (ready) { ", "cpp"], + ]) { + const expected = `${value}\n `; + assert.deepEqual( + indentNewline(value, value.length, value.length, language), + { value: expected, start: expected.length, end: expected.length } + ); + } + assert.deepEqual( + indentNewline(" case 1:", 11, 11, "javascript"), + { value: " case 1:\n ", start: 16, end: 16 } + ); +}); + +test("Enter does not add a level after comment-only lines", () => { + for (const [value, language] of [ + [" # Steps:", "python"], + [" // setup {", "javascript"], + [" /* setup {", "cpp"], + ]) { + const expected = `${value}\n `; + assert.deepEqual( + indentNewline(value, value.length, value.length, language), + { value: expected, start: expected.length, end: expected.length } + ); + } +}); + +test("Enter does not treat C++ preprocessor directives as comments", () => { + const value = " #define BLOCK {"; + const expected = `${value}\n `; + + assert.deepEqual( + indentNewline(value, value.length, value.length, "cpp"), + { value: expected, start: expected.length, end: expected.length } + ); +}); + +test("Enter ignores comment delimiters but keeps the code before and after comments", () => { + for (const [line, language, nested] of [ + ["work(); // {", "javascript", false], + ["value = 1 # note:", "python", false], + ["if ready: # explanation", "python", true], + ["if total // count: # integer division", "python", true], + ["if (ready) { // explanation", "javascript", true], + ["/* seed */ if (ready) {", "cpp", true], + ["if (ready) { /* explanation */", "java", true], + ["call(); /* { */", "c", false], + ["/* first */ /* second */", "cpp", false], + ["/* first */ // {", "cpp", false], + ["/* first */ /* second {", "cpp", false], + ['text = "# :"', "python", false], + ['const text = "// {"', "javascript", false], + ["const text = `/* {`", "javascript", false], + ]) { + const value = ` ${line}`; + const expected = `${value}\n${nested ? " " : " "}`; + assert.deepEqual( + indentNewline(value, value.length, value.length, language), + { value: expected, start: expected.length, end: expected.length }, line + ); + } +}); + +test("Enter keeps comment markers inside quoted strings", () => { + for (const [line, language] of [ + ['if url == "https://example.com": # note', "python"], + ["if tag == '#': # note", "python"], + ['if (url === "https://example.com") { // note', "javascript"], + ["if (tag === '/*') { /* note */", "javascript"], + ['if (tag === `//`) { // note', "javascript"], + [String.raw`if tag == 'can\'t #': # note`, "python"], + [String.raw`if (tag === "\"//") { // note`, "javascript"], + [String.raw`if path == "C:\\": # note`, "python"], + ]) { + const value = ` ${line}`; + const expected = `${value}\n `; + assert.deepEqual( + indentNewline(value, value.length, value.length, language), + { value: expected, start: expected.length, end: expected.length }, line + ); + } +}); + +test("Enter ignores a matching delimiter inside a trailing comment", () => { + const value = " work(); // {}"; + const start = value.length - 1; + assert.deepEqual(indentNewline(value, start, start, "javascript"), + { value: " work(); // {\n }", start: start + 5, end: start + 5 }); +}); + +test("Enter between matching delimiters leaves the caret on the inner line", () => { + for (const [open, close] of [["{", "}"], ["[", "]"], ["(", ")"]]) { + assert.deepEqual( + indentNewline(` ${open} ${close}`, 5, 5, "javascript"), + { value: ` ${open}\n \n ${close}`, start: 14, end: 14 } + ); + } + assert.deepEqual( + indentNewline("{\n}", 1, 1, "javascript"), + { value: "{\n \n}", start: 6, end: 6 } + ); + assert.deepEqual( + indentNewline("{]", 1, 1, "javascript"), + { value: "{\n ]", start: 6, end: 6 } + ); +}); + +test("Enter replaces a selection and preserves text on both sides", () => { + assert.deepEqual( + indentNewline(" beforeREMOVEafter", 10, 16, "javascript"), + { value: " before\n after", start: 15, end: 15 } + ); + assert.deepEqual( + indentNewline(" a\n b", 5, 9, "javascript"), + { value: " a\n ", start: 10, end: 10 } + ); + assert.deepEqual( + indentNewline(" abc", 2, 2, "javascript"), + { value: " \n abc", start: 5, end: 5 } + ); + assert.deepEqual( + indentNewline("\n abc", 0, 0, "javascript"), + { value: "\n\n abc", start: 1, end: 1 } + ); + assert.deepEqual( + indentNewline("", 0, 0, "python"), + { value: "\n", start: 1, end: 1 } + ); +}); test("Tab inserts four spaces on the current line or selected block", () => { assert.deepEqual(indentSelection("abc", 0, 0), { value: " abc", start: 4, end: 4 }); diff --git a/web/editor.js b/web/editor.js index dace8960..756f0d17 100644 --- a/web/editor.js +++ b/web/editor.js @@ -1,5 +1,60 @@ const INDENT = " "; +// Return the last non-whitespace code character on this line, ignoring +// comments while respecting quoted strings. No state carries across lines; +// multiline strings/comments and JavaScript regex literals need a tokenizer. +function lastCodeCharacter(line, language) { + let quote = ""; + let last = ""; + for (let index = 0; index < line.length; index += 1) { + const char = line[index]; + if (quote) { + if (char === "\\") index += 1; + else if (char === quote) quote = ""; + continue; + } + if (char === '"' || char === "'" || (language === "javascript" && char === "`")) { + quote = char; + } else if (language === "python") { + if (char === "#") break; + } else if (char === "/") { + if (line[index + 1] === "/") break; + if (line[index + 1] === "*") { + const end = line.indexOf("*/", index + 2); + if (end === -1) break; + index = end + 1; + continue; + } + } + if (char.trim()) last = char; + } + return last; +} + +export function indentNewline(value, start, end, language) { + const lineStart = start === 0 ? 0 : value.lastIndexOf("\n", start - 1) + 1; + const before = value.slice(lineStart, start); + const indentation = before.match(/^[ \t]*/)[0]; + const opener = lastCodeCharacter(before, language); + const closer = { "{": "}", "[": "]", "(": ")" }[opener]; + const nested = Boolean(closer) || (language === "python" && opener === ":"); + const innerIndent = indentation + (nested ? INDENT : ""); + let insertion = `\n${innerIndent}`; + const caret = start + insertion.length; + const after = value.slice(end); + const trailingSpace = after.match(/^[ \t]*/)[0].length; + // If the caret is between an opener and a closer, the indentation will be like this: + // + // if (a != b) { + // | <------- caret + // } + if (closer && after[trailingSpace] === closer) { + insertion += `\n${indentation}`; + end += trailingSpace; + } + return { value: value.slice(0, start) + insertion + value.slice(end), start: caret, end: caret }; +} + export function indentSelection(value, start, end, outdent = false) { // Not lastIndexOf alone: a negative fromIndex clamps to 0 and still matches // there, so a document that opens with a blank line would resolve the diff --git a/web/interview.js b/web/interview.js index 31201527..6ec6b15d 100644 --- a/web/interview.js +++ b/web/interview.js @@ -6,7 +6,7 @@ import { videoTrackReady, } from "./audio-check.js"; import { highlight } from "./highlight.js"; -import { indentSelection } from "./editor.js"; +import { indentNewline, indentSelection } from "./editor.js"; import { createDevicePool } from "./devices.js"; import { createFaceCheck } from "./face-check.js"; import { createMicMeter, startMediaMeter } from "./mic-meter.js"; @@ -469,6 +469,11 @@ function bindEvents() { event.preventDefault(); applyIndent(indentSelection(nodes.editor.value, nodes.editor.selectionStart, nodes.editor.selectionEnd, event.shiftKey)); }); + nodes.editor.addEventListener("beforeinput", (event) => { + if (event.inputType !== "insertLineBreak" || event.isComposing || !event.cancelable) return; + event.preventDefault(); + applyIndent(indentNewline(nodes.editor.value, nodes.editor.selectionStart, nodes.editor.selectionEnd, state.language)); + }); nodes.editor.addEventListener("input", () => { state.codeByLanguage[state.language] = nodes.editor.value; paintEditor();