Skip to content

Add auto indentation to the current code editor #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CX330Blake wants to merge 1 commit into
base: main
Choose a base branch
from
+178 −2
Open

Add auto indentation to the current code editor #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions scripts/browser-check.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -33,6 +34,44 @@ 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 " },
];
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");
Expand Down Expand Up @@ -533,6 +572,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
Expand Down
97 changes: 96 additions & 1 deletion tests/browser/editor.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,102 @@
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 }
);
}
});

@ColtenOuO ColtenOuO Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest adding some tests for the newly added comment handling.

ex.

["    # Steps:", "python"]
["    // setup {", "javascript"]
["if ready: # explain", "python"]
["run(); // setup {", "javascript"]

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 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 });
Expand Down
36 changes: 36 additions & 0 deletions web/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,41 @@
const INDENT = " ";

// TODO: The current implementation cannot handle cases like:

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repository carries no other TODO in first-party code, and this one records a limitation the pull request description already states, so once this merges it is a marker nothing tracks. Say what isCommentOnlyLine actually does instead: it classifies on the first token of a single line and carries no block comment state, which is why a continuation line inside /* ... */ reads as code.

// /*
// * comment
// */
Comment thread
CX330Blake marked this conversation as resolved.
function isCommentOnlyLine(line, language) {
if (language === "python") {
return /^[ \t]*#/.test(line);
}
return /^[ \t]*(\/\/|\/\*)/.test(line);

@ColtenOuO ColtenOuO Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isCommentOnlyLine() treats any C-like line starting with /* as comment-only, even when the comment closes and code follows.

For example, pressing Enter after /* guard */ if (ready) { preserves the current indentation instead of adding one level.

Could we either handle text after */ or explicitly track this case in #61?

@CX330Blake CX330Blake Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is not a common case people are likely to encounter in normal coding, I think we can leave it as a follow-up. I've added this case to #61 for tracking. Thanks for reviewing!

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches any line whose first token opens a block comment, even when the comment closes and real code follows, so /* seed */ if (ready) { is classified as a comment and the next line loses its level. Requiring the comment to stay open on the line keeps the intended cases and drops this one.

Suggested change
return /^[ \t]*(\/\/|\/\*)/.test(line);
return /^[ \t]*(\/\/|\/\*(?!.*\*\/))/.test(line);

}

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 = before.trimEnd().slice(-1);

@cubic-dev-ai cubic-dev-ai Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When Enter is pressed after a Python block with a trailing comment, this loses the required nested indentation because opener sees the comment text instead of the colon. Strip comments while respecting quoted strings, or use a syntax-aware scan before deciding whether the code prefix ends with a Python colon.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At web/editor.js, line 7:

<comment>When Enter is pressed after a Python block with a trailing comment, this loses the required nested indentation because `opener` sees the comment text instead of the colon. Strip comments while respecting quoted strings, or use a syntax-aware scan before deciding whether the code prefix ends with a Python colon.</comment>

<file context>
@@ -1,5 +1,29 @@
+  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 = before.trimEnd().slice(-1);
+  const closer = { "{": "}", "[": "]", "(": ")" }[opener];
+  const nested = Boolean(closer) || (language === "python" && opener === ":");
</file context>

@CX330Blake CX330Blake Sep 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This is indeed an edge case, but I think it's acceptable to leave it as-is for now. Supporting trailing Python comments correctly would require more syntax-aware parsing, and I'd prefer to avoid adding that complexity for a relatively uncommon case in this PR.

@ColtenOuO ColtenOuO Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine to handle as a follow-up, but I'd suggest mentioning it in the PR description for future reference.

@jserv jserv Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opener reads the raw line, so a trailing comment that ends in a delimiter or a colon adds a level the code never opened: work(); // { in JavaScript and value = 1 # note: in Python both indent the next line. The description covers the opposite direction (a real opener hidden before a trailing comment); this direction costs the candidate a deletion on every such line, so cutting the comment tail before reading the last token is worth doing here rather than in a follow-up.

const closer = { "{": "}", "[": "]", "(": ")" }[opener];
const comment = isCommentOnlyLine(before, language);
const nested = !comment && (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 (!comment && 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
Expand Down
7 changes: 6 additions & 1 deletion web/interview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
Expand Down
Loading