Skip to content

Let the candidate choose the editor indent width - #590

Open
princelu1009 wants to merge 1 commit into
sysprog21:mainfrom
princelu1009:editor-indent-width
Open

princelu1009 wants to merge 1 commit into
sysprog21:mainfrom
princelu1009:editor-indent-width

Conversation

@princelu1009

@princelu1009 princelu1009 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

The code editor always indented by four spaces (INDENT in web/editor.js), while 130 of the 151 JavaScript starters indent by two, so the first Tab or Enter inside a JavaScript starter function produced a body at a different width from the line above it, and a candidate who writes two-space or tab-indented code had no way to change it.

The lobby's editor options gain an Indentation select with 2 spaces, 4 spaces (the default) and Tab. The choice is saved in the browser and forwarded to the interview in a new editorIndent URL parameter. The interview nests by it until the buffer has an indented block of its own, then by the width that block uses (bufferIndent in web/indent-guides.js, which shares detectWidth's voting), so a four-space Python starter stays four-space under a two-space or tab choice instead of raising IndentationError or TabError; a space-indented line also nests with spaces under the tab unit. indentSelection and indentNewline take the unit as an argument, so Tab, Shift+Tab and auto-indent on Enter use it. Shift+Tab takes back one level: a single tab, or up to one unit's width of spaces, where the tab unit counts as four columns to match tab-size: 4, so four-space starter code still outdents by one level in one press.

Four spaces leaves no URL parameter and no stored value, so existing interview URLs behave as before, and the 66 existing tests in tests/browser/editor.test.js pass unmodified. The task page is unchanged, as agreed in #589: it honors none of the lobby's editor options today. The interview start payload pinned in tests/web/contract.rs does not change, since this is a display preference neither Jim nor the report needs.

Validation: new cases in tests/browser/editor.test.js (each unit for Tab, block indent and outdent, Enter after { and a Python :), tests/browser/editor-options.test.js (round-trip, unknown values fall back to four, storage), tests/browser/lobby.test.js (the select survives a reload, is disabled for the whiteboard and reaches the interview URL) tests/browser/editor-pipeline.test.js (the real interview page with editorIndent=2 and editorIndent=tab, and a four-space Python starter under each) and tests/browser/indent-guides.test.js (bufferIndent returns null with no block, and the width or a tab otherwise). Restoring the old editor.js, removing the unit arguments in web/interview.js, dropping bufferIndent, or dropping the space guard in indentNewline each fails a new test. ./scripts/test.sh on macOS passes everything this change touches; the lanes that fail (cargo-test's connection-drain timeout under load, task-authoring's timing ratio, run-budget, interactive-review-hook's /private/var path, and six git-hooks checks needing shellcheck, ruff or network) fail the same way on clean main here. Not run: a manual check in a live interview.

Close #589


Summary by cubic

Lets candidates pick 2 spaces, 4 spaces, or Tab for the editor's indentation in the lobby, instead of the fixed four spaces. The choice is saved in the browser and forwarded to the interview via an editorIndent URL parameter, and drives Tab, Shift+Tab, and auto-indent after Enter. Once the buffer has an indented block, nesting follows that block's own width, so a four-space Python starter stays four-space under a two-space or Tab choice rather than raising an IndentationError or TabError. Shift+Tab takes back one level, counting a Tab as four columns so four-space starter code still outdents correctly. Four spaces remains the default and leaves no URL parameter, so existing interview URLs and the task page behave exactly as before. Closes #589.

Written for commit cd50805. Summary will update on new commits.

View guided diff

cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv jserv added the explained A maintainer has seen the contributor explain this large change label Nov 5, 2026

@jserv jserv left a comment

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.

Rebase the current branch onto the upstream default branch and resolve any conflicts. Run /codetrial-review in AI agents (use $codetrial-review in Codex) and resolve all identified issues. Rework the series into minimal, logically coherent commits, squash closely related commits, and ensure all commit messages follow the project’s rules.

@ColtenOuO ColtenOuO left a comment

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.

Please provide some screenshots of your changes (some frontend changes in this PR)

Comment thread web/interview.js Resolved
Comment thread web/editor.js Resolved
Comment thread web/editor.js Resolved
cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv jserv left a comment

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.

Rebase the current branch onto the upstream default branch and resolve any conflicts. Run /codetrial-review in AI agents (use $codetrial-review in Codex) and resolve all identified issues. Rework the series into minimal, logically coherent commits, squash closely related commits, and ensure all commit messages follow the project’s rules.

@princelu1009

princelu1009 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor Author

@ColtenOuO Screenshots at a155276:

Lobby: the new Indentation choice in Editor options

Lobby editor options with Indentation set to 2 spaces

JavaScript with 2 spaces: Enter after { nests by two at each level

JavaScript editor nesting by two spaces

The editor always indented by four spaces, while 130 of the 151
JavaScript starters indent by two, so the first Tab or Enter inside a
starter function broke its indentation and a candidate had no way to
pick another width.

The lobby's editor options gain an Indentation choice of two spaces,
four spaces or a tab. The interview nests by it until the buffer has an
indented block of its own, then by the width that block uses, so a
four-space Python starter stays four-space under a two-space or tab
choice instead of raising IndentationError or TabError. Shift+Tab takes
back one unit. Four spaces stays the default and leaves no URL
parameter, so existing interview URLs and the task page behave as
before.

Close sysprog21#589

Change-Id: I55a217cf1bc6a5f5555c3b0d7389c14fb3304c05
@jserv

jserv commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Screenshots at a155276:
Lobby: the new Indentation choice in Editor options

Include screenshots in the pull request description rather than in the comments.

Comment thread web/interview.js
/// written in another style keeps its own.
function indentFor(code) {
return (
bufferIndent(code, editorTokens(code, state.language).tokens) ?? indentUnit

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.

indentFor re-measures the buffer on every keystroke, so the unit changes as soon as Tab edits the buffer. In a JavaScript starter whose body is a single two-space comment line, the width votes 2. Tab then makes that line 4 columns, the next call votes 4, and Shift+Tab removes up to 4 spaces, leaving the line at column 0 instead of back at 2. A Python buffer with one body line (if True: / pass) loses its indentation the same way, and a four-space one goes to 8 and back to 0. Keep the inferred unit stable across ordinary edits: infer it when the starter loads, the buffer is restored or the language changes, and reuse it until then. Add a test that presses Tab then Shift+Tab through indentFor on a two-space starter; the pipeline tests pass a fixed unit and cannot see this.

Comment thread web/indent-guides.js
if (whitespace.startsWith("\t")) tabs += 1;
else if (whitespace.startsWith(" ")) spaces += 1;
}
return tabs > spaces ? "\t" : " ".repeat(width);

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.

tabs > spaces is a vote over the whole buffer, so it also decides how Shift+Tab treats a suite indented the other way. With one function indented by tabs and a longer one indented by spaces (valid Python), the unit comes out as "\t" and Shift+Tab on a four-space line removes four spaces, which can take a line out of its suite. Decide the unit from the line being edited or the suite around it, and fall back to the global vote only when that line has no indentation to read. Add a case with separately tab-indented and space-indented functions.

Comment thread web/editor.js
};
}
// One level back: a single tab, or up to one unit's width of spaces.
const level = new RegExp(

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.

With the tab unit the level pattern is ^\t|^ {1,4}, so for a line indented \t (a space, then a tab) Shift+Tab strips only the space and the line stays four columns deep. This is inherited from the old ^\t|^ {1,4} but the tab option makes mixed prefixes easier to reach. Follow-up: remove the leading whitespace by columns up to one level (TAB_COLUMNS for the tab unit), and add a test for \tpass.

Comment thread web/editor-options.js
}

export function saveIndent(name, storage) {
writeStored(INDENT_STORAGE_KEY, name === DEFAULT_INDENT ? "" : name, storage);

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.

loadIndent and indentFromParams check the value against INDENT_UNITS, but saveIndent writes whatever the caller passes. Only the lobby's select calls it today, so nothing is wrong yet; normalize here as well, so the stored value can only ever be one the readers accept, and add a test for refused storage on the new helpers.

@jserv jserv left a comment

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.

Rebase the current branch onto the upstream default branch and resolve any conflicts. Run /codetrial-review in AI agents (use $codetrial-review in Codex) and resolve all identified issues. Rework the series into minimal, logically coherent commits, squash closely related commits, and ensure all commit messages follow the project’s rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

explained A maintainer has seen the contributor explain this large change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Let the candidate choose the editor indentation width

3 participants