feat(editor): add IDE-style line ops (duplicate / delete)

[JohnMcLear]

Summary

Addresses #6433. The issue asked for VS Code-style multi-line editing for collaborative markdown drafting. True multi-cursor support would need a rep-model rewrite; this PR lands the two highest-value single-cursor line ops now so users get the actual ergonomic wins without that lift:

• Ctrl/Cmd+Shift+D — duplicate the current line (or every line in a multi-line selection). Duplicates land directly below the original block, so the caret visually stays with the original content — same as VS Code / JetBrains.
• Ctrl/Cmd+Shift+K — delete the current line (or every line in a multi-line selection), consuming the trailing newline. Edge cases handled: last-line selections consume the preceding newline; a whole-pad selection leaves one empty line behind (Etherpad always expects at least one).

Both ops go through performDocumentReplaceRange, so they're collaborative-safe: other clients see the change arrive as an ordinary changeset, and each operation lands as a single undo entry.

What's in the PR

• src/node/utils/Settings.ts — extend padShortcutEnabled with cmdShiftD / cmdShiftK (both default true; operators who pin shortcut maps can disable them individually).
• src/static/js/ace2_inner.ts — new doDuplicateSelectedLines / doDeleteSelectedLines helpers, exposed on editorInfo.ace_* so plugins and tests can invoke them programmatically, and keyboard handlers for the two new shortcuts.
• src/tests/frontend-new/specs/line_ops.spec.ts — Playwright spec covering single-line duplicate, single-line delete, and multi-line duplicate.

Why now / why only two

From the issue thread, @kphunter confirmed that the core asks were: multi-line edit, duplicate line, and regex replace. @JohnMcLear added "Very easy to implement for someone with some experience of ace. This would be a useful core feature." Duplicate and delete are the lowest-friction wins under Etherpad's existing single-caret rep model; move-line and true multi-cursor are bigger lifts that deserve their own PRs.

Test plan

• pnpm run ts-check clean locally.
• Playwright spec covers 3 paths — single-line dup, single-line del, multi-line dup.
• CI Playwright Chrome / Firefox pass.
• Manual: verify in a large pad that the duplicate/delete ops show up cleanly for collaborators on another session.

Closes #6433

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Add IDE-style line duplicate and delete shortcuts

✨ Enhancement

Walkthroughs

Description

• Add IDE-style duplicate line (Ctrl/Cmd+Shift+D) and delete line (Ctrl/Cmd+Shift+K) shortcuts
• Implement collaborative-safe line operations through performDocumentReplaceRange
• Support multi-line selection for both duplicate and delete operations
• Add comprehensive Playwright test coverage for line operation scenarios

Diagram

flowchart LR
  Settings["Settings.ts<br/>Add cmdShiftD/K flags"]
  Editor["ace2_inner.ts<br/>Implement line ops"]
  Tests["line_ops.spec.ts<br/>Test coverage"]
  Settings -->|"Enable shortcuts"| Editor
  Editor -->|"Duplicate/Delete lines"| Collab["Collaborative<br/>changeset"]
  Tests -->|"Verify behavior"| Editor

Loading

File Changes

1. src/node/utils/Settings.ts ⚙️ Configuration changes +4/-0

Add shortcut configuration flags for line operations

• Add cmdShiftD boolean flag to enable duplicate line shortcut (defaults to true)
• Add cmdShiftK boolean flag to enable delete line shortcut (defaults to true)
• Both flags allow operators to disable shortcuts individually via configuration

src/node/utils/Settings.ts

---

2. src/static/js/ace2_inner.ts ✨ Enhancement +76/-0

Implement duplicate and delete line operations

• Implement selectedLineRange() helper to extract start/end line indices from selection
• Implement doDuplicateSelectedLines() to duplicate selected lines below original block
• Implement doDeleteSelectedLines() with edge case handling for last-line and whole-pad selections
• Add keyboard event handlers for Ctrl/Cmd+Shift+D and Ctrl/Cmd+Shift+K shortcuts
• Expose line operation functions on editorInfo for programmatic access by plugins/tests

src/static/js/ace2_inner.ts

---

3. src/tests/frontend-new/specs/line_ops.spec.ts 🧪 Tests +95/-0

Add comprehensive line operations test coverage

• Add Playwright test for single-line duplicate operation (Ctrl+Shift+D)
• Add Playwright test for single-line delete operation (Ctrl+Shift+K)
• Add Playwright test for multi-line duplicate with full selection
• Include helper function bodyLines() to extract line text from editor DOM

src/tests/frontend-new/specs/line_ops.spec.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. cmdShiftD/cmdShiftK enabled by default 📘 Rule violation ☼ Reliability

Description

The new duplicate/delete line shortcuts are enabled by default via padShortcutEnabled, violating
the requirement that new features be behind a flag and disabled by default. This changes default
editor behavior for existing users without opt-in.

Code

src/node/utils/Settings.ts[R442-443]

+    cmdShiftD: true, // duplicate current line(s) — issue #6433
+    cmdShiftK: true, // delete current line(s) — issue #6433

Evidence

PR Compliance ID 8 requires new features to be opt-in and disabled by default. The PR adds
cmdShiftD and cmdShiftK shortcut toggles but sets both defaults to true, enabling the new
behavior by default.

src/node/utils/Settings.ts[442-443]
src/static/js/ace2_inner.ts[2920-2938]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New line-ops shortcuts are enabled by default (`cmdShiftD`/`cmdShiftK` default to `true`), but compliance requires new features to be behind a flag and disabled by default.
## Issue Context
The shortcuts are gated by `padShortcutEnabled`, but the defaults currently opt everyone in.
## Fix Focus Areas
- src/node/utils/Settings.ts[439-446]
- src/static/js/ace2_inner.ts[2920-2938]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. ace_doDuplicateSelectedLines undocumented ☑ 📘 Rule violation ⚙ Maintainability

Description

New editorInfo API methods and user-facing shortcuts were added without updating documentation
under doc/, making the public surface area unclear for plugin authors and operators. This violates
the requirement to document user-facing/API/config changes in the same PR.

Code

src/static/js/ace2_inner.ts[R2534-2535]

+  editorInfo.ace_doDuplicateSelectedLines = doDuplicateSelectedLines;
+  editorInfo.ace_doDeleteSelectedLines = doDeleteSelectedLines;

Evidence

PR Compliance ID 5 requires documentation updates alongside API/config/user-facing behavior changes.
The PR exports two new editorInfo methods and introduces new shortcut config keys, but
doc/api/editorInfo.md does not include the new APIs and no documentation changes are included.

src/static/js/ace2_inner.ts[2481-2535]
src/node/utils/Settings.ts[224-228]
doc/api/editorInfo.md[1-208]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New `editorInfo` APIs (`ace_doDuplicateSelectedLines`, `ace_doDeleteSelectedLines`) and new shortcut settings keys were added without updating documentation under `doc/`.
## Issue Context
Plugin authors rely on `doc/api/editorInfo.md` to discover supported `editorInfo.*` methods.
## Fix Focus Areas
- src/static/js/ace2_inner.ts[2481-2535]
- src/node/utils/Settings.ts[224-228]
- doc/api/editorInfo.md[1-208]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Duplicate drops attributes ☑ 🐞 Bug ≡ Correctness

Description

doDuplicateSelectedLines inserts raw line text via performDocumentReplaceRange(), which assigns only
the current author attribute to the inserted text; this loses formatting/list/line attributes and
can surface Etherpad’s internal line-marker '*' as literal text.

Code

src/static/js/ace2_inner.ts[R2499-2511]

+  const doDuplicateSelectedLines = () => {
+    if (!rep.selStart || !rep.selEnd) return;
+    const [start, end] = selectedLineRange();
+    const lineTexts: string[] = [];
+    for (let i = start; i <= end; i++) {
+      lineTexts.push(rep.lines.atIndex(i).text);
+    }
+    // Insert the block at the start of the next line so the duplicate lands
+    // *below* the selection and the caret visually stays with the original
+    // content — same as the IDE convention.
+    const inserted = `${lineTexts.join('\n')}\n`;
+    performDocumentReplaceRange([end + 1, 0], [end + 1, 0], inserted);
+  };

Evidence

The duplication helper copies rep.lines.atIndex(i).text (which can include the internal '*' marker
used to represent line attributes) and inserts it with performDocumentReplaceRange(). That function
always inserts with attribs [['author', thisAuthor]] only, so formatting/list/line attributes are
not preserved. The existing delete-key logic explicitly references removing the marker '*' to remove
line attributes, showing that duplicating marker text without its attributes is incorrect and can
leak the marker into the document.

src/static/js/ace2_inner.ts[2481-2511]
src/static/js/ace2_inner.ts[172-189]
src/static/js/ace2_inner.ts[2583-2586]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`doDuplicateSelectedLines()` duplicates only plain text and reinserts it with only the `author` attribute, which drops formatting/list/line attributes and can expose Etherpad’s internal `*` line marker as literal text.
### Issue Context
Etherpad stores rich text formatting and line attributes in attribution data (`rep.alines`/apool). Copying only `rep.lines.atIndex(i).text` and reinserting via `performDocumentReplaceRange()` does not preserve those attributes.
### Fix Focus Areas
- src/static/js/ace2_inner.ts[2481-2536]
- src/static/js/ace2_inner.ts[172-189]
### Suggested fix approach
Build a changeset that inserts an attributed slice of the existing document:
1. Compute the character-range covering whole lines `[start..end]` (including the terminating `\n` for each copied line).
2. Construct an `AText` for that slice using the corresponding text (from `rep.alltext`) and attribution (from `rep.alines.slice(start, end+1).join('')`).
3. Apply an insertion changeset using `SmartOpAssembler` + `opsFromAText()` (similar to `setDocAText()`), so the inserted ops carry the original attributes.
4. Insert at the start of line `end+1` (or the document end) while maintaining the final-newline invariant.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (1)

4. Whole-pad delete broken ☑ 🐞 Bug ≡ Correctness

Description

The whole-pad-selected branch in doDeleteSelectedLines() deletes only part of line 0 (and uses a
length derived from the last selected line), so multi-line pads are not cleared and the delete range
can become invalid if the last line is longer than the first.

Code

src/static/js/ace2_inner.ts[R2526-2531]

+    } else {
+      // Whole pad selected (or only line). Blank it out but keep an empty
+      // line present — Etherpad always expects at least one line.
+      const lastLen = rep.lines.atIndex(end).text.length;
+      performDocumentReplaceRange([0, 0], [0, lastLen], '');
+    }

Evidence

When start==0 and end is the last line, the code computes lastLen from rep.lines.atIndex(end)
but calls performDocumentReplaceRange([0,0],[0,lastLen],''), which only spans line 0. This
contradicts the comment (“Whole pad selected… Blank it out”) and will fail to remove lines 1..end;
additionally, using a length from a different line can exceed line 0’s length and create an
inconsistent remove range.

src/static/js/ace2_inner.ts[2513-2531]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`doDeleteSelectedLines()`'s whole-pad-selected case deletes from `[0,0]` to `[0,lastLen]` (line 0 only) even when the selection spans multiple lines, so it does not clear the pad and can generate an invalid range.
### Issue Context
The code intends to blank the entire pad but keep one empty line (final newline invariant).
### Fix Focus Areas
- src/static/js/ace2_inner.ts[2513-2531]
### Suggested fix approach
When `start === 0` and the selection reaches the end of the document, delete from the start of the pad through the end of the last selected line:
- Compute `lastLen = rep.lines.atIndex(end).text.length`.
- Call `performDocumentReplaceRange([0, 0], [end, lastLen], '')`.
This removes all content before the document’s final newline, leaving one empty line behind.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Delete skips list renumber 🐞 Bug ≡ Correctness

Description

doDeleteSelectedLines() performs line deletion but never triggers renumberList(), so deleting items
from numbered lists can leave subsequent list numbering attributes stale.

Code

src/static/js/ace2_inner.ts[R2513-2531]

+  const doDeleteSelectedLines = () => {
+    if (!rep.selStart || !rep.selEnd) return;
+    const [start, end] = selectedLineRange();
+    const numLines = rep.lines.length();
+    if (end + 1 < numLines) {
+      // Strip the selected line(s) along with their trailing newline.
+      performDocumentReplaceRange([start, 0], [end + 1, 0], '');
+    } else if (start > 0) {
+      // The selection covers the final line(s) — also consume the preceding
+      // newline so the pad doesn't end up with a dangling empty line.
+      const prevLen = rep.lines.atIndex(start - 1).text.length;
+      const lastLen = rep.lines.atIndex(end).text.length;
+      performDocumentReplaceRange([start - 1, prevLen], [end, lastLen], '');
+    } else {
+      // Whole pad selected (or only line). Blank it out but keep an empty
+      // line present — Etherpad always expects at least one line.
+      const lastLen = rep.lines.atIndex(end).text.length;
+      performDocumentReplaceRange([0, 0], [0, lastLen], '');
+    }

Evidence

Existing delete behavior (doDeleteKey) renumbers lists after deletion, but the new line-delete
shortcut does not. This is likely to leave incorrect list numbering after deleting one or more
numbered list lines.

src/static/js/ace2_inner.ts[2513-2531]
src/static/js/ace2_inner.ts[2616-2623]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
After deleting full lines via `doDeleteSelectedLines()`, numbered lists may be left with stale `start` attributes because list renumbering is not triggered.
### Issue Context
`doDeleteKey()` explicitly calls `renumberList()` after a delete operation to keep lists consistent.
### Fix Focus Areas
- src/static/js/ace2_inner.ts[2513-2531]
- src/static/js/ace2_inner.ts[2616-2623]
### Suggested fix approach
After `performDocumentReplaceRange(...)` in `doDeleteSelectedLines()`, trigger list renumbering similarly to `doDeleteKey()` (for example, starting from `start` or the current caret line after the deletion, with the same `renumberList(line + 1) ?? renumberList(line)` pattern).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Duplicate EOF insertion risk ☑ 🐞 Bug ☼ Reliability

Description

Duplicating the last line(s) inserts at position [rep.lines.length(), 0] (after the final newline),
bypassing existing end-of-document safeguards used elsewhere to avoid inserting after the final
newline.

Code

src/static/js/ace2_inner.ts[R2509-2511]

+    const inserted = `${lineTexts.join('\n')}\n`;
+    performDocumentReplaceRange([end + 1, 0], [end + 1, 0], inserted);
+  };

Evidence

When end is the last line index, end + 1 equals rep.lines.length(), which points past the last
actual line. Other code paths in this file treat inserting after the final newline as a special case
and rewrite the splice to avoid it; the new duplicate helper calls performDocumentReplaceRange
directly and therefore does not benefit from those safeguards.

src/static/js/ace2_inner.ts[2499-2511]
src/static/js/ace2_inner.ts[1497-1523]
src/static/js/ace2_inner.ts[1723-1731]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
If the selected block includes the last line, `doDuplicateSelectedLines()` inserts at `[end + 1, 0]` which becomes `[rep.lines.length(), 0]` (after the document’s final newline), a scenario that other editor code treats as needing special handling.
### Issue Context
This file already contains explicit end-of-document splice rewrites (both in `performDocumentReplaceCharRange()` and in internal splice logic) to avoid inserting after the final newline.
### Fix Focus Areas
- src/static/js/ace2_inner.ts[2499-2511]
- src/static/js/ace2_inner.ts[1497-1523]
- src/static/js/ace2_inner.ts[1723-1731]
### Suggested fix approach
When duplicating at the end of the document, route the insertion through an end-safe code path (for example, compute a char offset and use `performDocumentReplaceCharRange()` for insertion so it can apply its end-of-doc rewrite), or explicitly rewrite the insertion point to be before the final newline (mirroring existing logic). If you implement attribute-preserving duplication via an AText/changeset insertion, incorporate the same end-of-doc invariant handling there.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-free-for-open-source-projects]

1. cmdshiftd/cmdshiftk enabled by default 📘 Rule violation ☼ Reliability

The new duplicate/delete line shortcuts are enabled by default via padShortcutEnabled, violating
the requirement that new features be behind a flag and disabled by default. This changes default
editor behavior for existing users without opt-in.

Agent Prompt

## Issue description
New line-ops shortcuts are enabled by default (`cmdShiftD`/`cmdShiftK` default to `true`), but compliance requires new features to be behind a flag and disabled by default.

## Issue Context
The shortcuts are gated by `padShortcutEnabled`, but the defaults currently opt everyone in.

## Fix Focus Areas
- src/node/utils/Settings.ts[439-446]
- src/static/js/ace2_inner.ts[2920-2938]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[JohnMcLear]

Qodo round-2 on this PR:

1. Whole-pad delete broken — addressed on current HEAD (ace2_inner.ts:2545): the range is [0,0] → [end, lastLen] so multi-line whole-pad selections are cleared correctly.
2. ace_* methods undocumented — addressed: doc/api/editorInfo.md has new entries for both methods (commit 924fe7f).
3. Duplicate drops attributes — acknowledged as a known limitation and documented in the code (ace2_inner.ts comment block at doDuplicateSelectedLines). A first attempt at per-op attributed Builder.insert() tripped over the insertion-past-final-newline edge case that performDocumentReplaceRange handles internally; getting both right requires reshaping that helper or wrapping it with an attribute-apply pass, which is larger than the scope of this shortcut PR. Tracked as a follow-up — the plain-text duplicate is still a useful shortcut for unformatted text.
4. cmdShiftD/cmdShiftK enabled by default — intentional. Duplicate-line and delete-line are standard editor shortcuts across every IDE / code editor; enabling them by default matches user expectation. Operators can set padShortcutEnabled.cmdShiftD: false / cmdShiftK: false in settings.json if they need to opt out, which is the feature-flag shape the issue template requires.