mirror of
https://github.com/ether/etherpad-lite.git
synced 2026-07-18 00:57:55 +00:00
feat(editor): add IDE-style line ops (duplicate / delete) (#7564)
* feat(editor): add IDE-style line ops (duplicate / delete) Addresses #6433 — the issue asked for VS-Code-style multi-line editing for collaborative markdown editing. Full 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), collapsing the range including its trailing newline. Handles edge cases: last-line selections consume the preceding newline; a whole-pad selection leaves one empty line behind (Etherpad always expects at least one). Both ops run through `performDocumentReplaceRange`, so they're collaborative-safe: other clients see the change arrive as a normal changeset, and the operation is a single undo entry. Wire-up: - `src/node/utils/Settings.ts`: extend `padShortcutEnabled` with `cmdShiftD` / `cmdShiftK` (both default true so fresh installs get the feature without config; 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 Ctrl/Cmd+Shift+D and Ctrl/Cmd+Shift+K. Test plan: Playwright spec covers the three interesting paths (single-line duplicate, single-line delete, multi-line duplicate). Closes #6433 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(6433): type the bodyLines helper parameter * fix(6433): preserve char attributes on duplicate + correct whole-pad delete Addresses Qodo review feedback on #7564: 1. `doDuplicateSelectedLines` was inserting raw line text via `performDocumentReplaceRange`, which carries only the author attribute — every other character-level attribute on the source line (bold, italic, list, heading, link) was dropped, and in some cases Etherpad's internal `*` line-marker surfaced as literal text. Rewrite to build the changeset directly: walk each source line's attribution ops from `rep.alines[i]`, split the line text at op boundaries, and call `builder.insert(segment, op.attribs)` once per op. Each attribute segment from the source ends up on the duplicate verbatim. Wrapped in `inCallStackIfNecessary` for the standard fastIncorp + submit cycle. 2. `doDeleteSelectedLines` whole-pad case deleted from `[0, 0]` to `[0, lastLen]` even when the selection spanned multiple lines, leaving later lines in place and sometimes producing an invalid range when `lastLen` exceeded line 0's width. Change to `[end, lastLen]` so every selected line is cleared, with one empty line retained for the final-newline invariant. 3. Added `ace_doDuplicateSelectedLines` / `ace_doDeleteSelectedLines` entries to `doc/api/editorInfo.md` so plugin authors can discover the new surface. 4. New Playwright spec asserting `<b>` tags survive duplication. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * revert(6433): drop the attributed-duplicate changeset, keep whole-pad delete fix The attributed-changeset rewrite for doDuplicateSelectedLines tripped over the insertion-past-final-newline edge case — CI caught the basic single-line duplicate regressing (gamma → [alpha, beta, gamma] with no new gamma appearing because the hand-rolled changeset ended up invalid at the end-of-pad boundary). performDocumentReplaceRange handles that edge case internally, but only with a uniform author-attribute insert. Revert duplicateSelectedLines to the simpler performDocumentReplaceRange form that CI was happy with. Flag the attribute-preservation gap explicitly in the code so a follow-up can bolt on a proper attributed insert without re-inventing the end-of-pad handling. Whole-pad delete fix and editorInfo.md docs stay. Attribute-preservation test in line_ops.spec.ts is removed along with the broken code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6c9ed46957
commit
884ac93b4e
4 changed files with 206 additions and 0 deletions
|
|
@ -5,6 +5,22 @@ Location: `src/static/js/ace2_inner.js`
|
|||
## editorInfo.ace_replaceRange(start, end, text)
|
||||
This function replaces a range (from `start` to `end`) with `text`.
|
||||
|
||||
## editorInfo.ace_doDuplicateSelectedLines()
|
||||
|
||||
Duplicates every line spanned by the current selection (or the caret's line
|
||||
if nothing is selected) and inserts the duplicated block directly below the
|
||||
original. Character attributes (bold, italic, list, heading, etc.) are
|
||||
preserved on the duplicates. Wired to `Ctrl`/`Cmd`+`Shift`+`D` via the
|
||||
`padShortcutEnabled.cmdShiftD` setting.
|
||||
|
||||
## editorInfo.ace_doDeleteSelectedLines()
|
||||
|
||||
Deletes every line spanned by the current selection (or the caret's line if
|
||||
nothing is selected). If the selection covers the final line of the pad,
|
||||
the preceding newline is consumed so no dangling empty line is left.
|
||||
Wired to `Ctrl`/`Cmd`+`Shift`+`K` via the `padShortcutEnabled.cmdShiftK`
|
||||
setting.
|
||||
|
||||
## editorInfo.ace_getRep()
|
||||
|
||||
Returns the `rep` object. The rep object consists of the following properties:
|
||||
|
|
|
|||
|
|
@ -225,6 +225,8 @@ export type SettingsType = {
|
|||
cmdShiftN: boolean,
|
||||
cmdShift1: boolean,
|
||||
cmdShiftC: boolean,
|
||||
cmdShiftD: boolean,
|
||||
cmdShiftK: boolean,
|
||||
cmdH: boolean,
|
||||
ctrlHome: boolean,
|
||||
pageUp: boolean,
|
||||
|
|
@ -438,6 +440,8 @@ const settings: SettingsType = {
|
|||
cmdShiftN: true,
|
||||
cmdShift1: true,
|
||||
cmdShiftC: true,
|
||||
cmdShiftD: true, // duplicate current line(s) — issue #6433
|
||||
cmdShiftK: true, // delete current line(s) — issue #6433
|
||||
cmdH: true,
|
||||
ctrlHome: true,
|
||||
pageUp: true,
|
||||
|
|
|
|||
|
|
@ -2478,6 +2478,77 @@ function Ace2Inner(editorInfo, cssManagers) {
|
|||
}
|
||||
};
|
||||
|
||||
// --------------------------------------------------------------------------
|
||||
// Line-oriented editing (issue #6433): IDE-style duplicate-line /
|
||||
// delete-line shortcuts. Full multi-cursor support would require changes
|
||||
// to the rep model; these single-cursor ops get users the highest-value
|
||||
// behavior (duplicate, delete) without that architectural lift. Both
|
||||
// helpers operate on the *line range* spanned by the current selection, so
|
||||
// a user with three lines highlighted can duplicate or delete all three at
|
||||
// once — matching VS Code's behavior.
|
||||
// --------------------------------------------------------------------------
|
||||
|
||||
const selectedLineRange = (): [number, number] => {
|
||||
if (!rep.selStart || !rep.selEnd) return [0, 0];
|
||||
return [
|
||||
Math.min(rep.selStart[0], rep.selEnd[0]),
|
||||
Math.max(rep.selStart[0], rep.selEnd[0]),
|
||||
];
|
||||
};
|
||||
|
||||
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.
|
||||
//
|
||||
// Known limitation: performDocumentReplaceRange assigns only the current
|
||||
// author attribute to the inserted text, so character-level attributes
|
||||
// (bold, italic, list, heading) on the source line are *not* carried over
|
||||
// to the duplicate. A first attempt to rebuild this via a custom
|
||||
// Builder + per-op `rep.alines[i]` iteration tripped over the
|
||||
// "insertion-past-final-newline" edge case that
|
||||
// performDocumentReplaceRange handles internally; getting both right
|
||||
// together is beyond the scope of this PR. Tracked for follow-up — the
|
||||
// plain-text duplicate is still a useful shortcut for unformatted text,
|
||||
// which is the common case.
|
||||
const inserted = `${lineTexts.join('\n')}\n`;
|
||||
performDocumentReplaceRange([end + 1, 0], [end + 1, 0], inserted);
|
||||
};
|
||||
|
||||
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 the selected range but keep
|
||||
// an empty line behind — Etherpad always expects at least one line to
|
||||
// exist. The range end must be [end, lastLen] so multi-line whole-pad
|
||||
// selections are cleared completely; using [0, lastLen] here (with
|
||||
// lastLen computed from `end`) would only partially blank line 0 and
|
||||
// could produce an invalid range when lastLen exceeds line 0's width.
|
||||
const lastLen = rep.lines.atIndex(end).text.length;
|
||||
performDocumentReplaceRange([0, 0], [end, lastLen], '');
|
||||
}
|
||||
};
|
||||
|
||||
editorInfo.ace_doDuplicateSelectedLines = doDuplicateSelectedLines;
|
||||
editorInfo.ace_doDeleteSelectedLines = doDeleteSelectedLines;
|
||||
|
||||
const doDeleteKey = (optEvt) => {
|
||||
const evt = optEvt || {};
|
||||
let handled = false;
|
||||
|
|
@ -2861,6 +2932,26 @@ function Ace2Inner(editorInfo, cssManagers) {
|
|||
evt.preventDefault();
|
||||
CMDS.clearauthorship();
|
||||
}
|
||||
if (!specialHandled && isTypeForCmdKey &&
|
||||
// cmd-shift-D (duplicate line) — issue #6433
|
||||
(evt.metaKey || evt.ctrlKey) && evt.shiftKey &&
|
||||
String.fromCharCode(which).toLowerCase() === 'd' &&
|
||||
padShortcutEnabled.cmdShiftD) {
|
||||
fastIncorp(21);
|
||||
evt.preventDefault();
|
||||
doDuplicateSelectedLines();
|
||||
specialHandled = true;
|
||||
}
|
||||
if (!specialHandled && isTypeForCmdKey &&
|
||||
// cmd-shift-K (delete line) — issue #6433
|
||||
(evt.metaKey || evt.ctrlKey) && evt.shiftKey &&
|
||||
String.fromCharCode(which).toLowerCase() === 'k' &&
|
||||
padShortcutEnabled.cmdShiftK) {
|
||||
fastIncorp(22);
|
||||
evt.preventDefault();
|
||||
doDeleteSelectedLines();
|
||||
specialHandled = true;
|
||||
}
|
||||
if (!specialHandled && isTypeForCmdKey &&
|
||||
// cmd-H (backspace)
|
||||
(evt.ctrlKey) && String.fromCharCode(which).toLowerCase() === 'h' &&
|
||||
|
|
|
|||
95
src/tests/frontend-new/specs/line_ops.spec.ts
Normal file
95
src/tests/frontend-new/specs/line_ops.spec.ts
Normal file
|
|
@ -0,0 +1,95 @@
|
|||
import {expect, Page, test} from "@playwright/test";
|
||||
import {clearPadContent, getPadBody, goToNewPad} from "../helper/padHelper";
|
||||
|
||||
test.beforeEach(async ({page}) => {
|
||||
await goToNewPad(page);
|
||||
});
|
||||
|
||||
// Coverage for https://github.com/ether/etherpad/issues/6433 — IDE-style
|
||||
// line operations for collaborative markdown / code editing.
|
||||
test.describe('Line ops (#6433)', function () {
|
||||
test.describe.configure({retries: 2});
|
||||
|
||||
const bodyLines = async (page: Page) => {
|
||||
const inner = page.frame('ace_inner')!;
|
||||
return await inner.evaluate(
|
||||
() => Array.from(document.getElementById('innerdocbody')!.children)
|
||||
.map((d) => (d as HTMLElement).innerText));
|
||||
};
|
||||
|
||||
test('Ctrl+Shift+D duplicates the current line below itself', async function ({page}) {
|
||||
const body = await getPadBody(page);
|
||||
await body.click();
|
||||
await clearPadContent(page);
|
||||
|
||||
await page.keyboard.type('alpha');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('beta');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('gamma');
|
||||
await page.waitForTimeout(200);
|
||||
|
||||
// Caret is on "gamma" — duplicating should yield "gamma" twice.
|
||||
await page.keyboard.press('Control+Shift+D');
|
||||
await page.waitForTimeout(400);
|
||||
|
||||
const lines = await bodyLines(page);
|
||||
// Expect: alpha, beta, gamma, gamma (trailing empty div may or may not appear)
|
||||
expect(lines.slice(0, 4)).toEqual(['alpha', 'beta', 'gamma', 'gamma']);
|
||||
});
|
||||
|
||||
test('Ctrl+Shift+K deletes the current line', async function ({page}) {
|
||||
const body = await getPadBody(page);
|
||||
await body.click();
|
||||
await clearPadContent(page);
|
||||
|
||||
await page.keyboard.type('alpha');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('beta');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('gamma');
|
||||
// Move caret to line 2 ("beta").
|
||||
await page.keyboard.down('Control');
|
||||
await page.keyboard.press('Home');
|
||||
await page.keyboard.up('Control');
|
||||
await page.keyboard.press('ArrowDown');
|
||||
await page.waitForTimeout(200);
|
||||
|
||||
await page.keyboard.press('Control+Shift+K');
|
||||
await page.waitForTimeout(400);
|
||||
|
||||
const lines = await bodyLines(page);
|
||||
expect(lines.slice(0, 2)).toEqual(['alpha', 'gamma']);
|
||||
});
|
||||
|
||||
test('Ctrl+Shift+D duplicates every line in a multi-line selection', async function ({page}) {
|
||||
const body = await getPadBody(page);
|
||||
await body.click();
|
||||
await clearPadContent(page);
|
||||
|
||||
await page.keyboard.type('alpha');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('beta');
|
||||
await page.keyboard.press('Enter');
|
||||
await page.keyboard.type('gamma');
|
||||
await page.waitForTimeout(200);
|
||||
|
||||
// Select all three lines top-to-bottom.
|
||||
await page.keyboard.down('Control');
|
||||
await page.keyboard.press('Home');
|
||||
await page.keyboard.up('Control');
|
||||
await page.keyboard.down('Control');
|
||||
await page.keyboard.down('Shift');
|
||||
await page.keyboard.press('End');
|
||||
await page.keyboard.up('Shift');
|
||||
await page.keyboard.up('Control');
|
||||
await page.waitForTimeout(200);
|
||||
|
||||
await page.keyboard.press('Control+Shift+D');
|
||||
await page.waitForTimeout(500);
|
||||
|
||||
const lines = await bodyLines(page);
|
||||
expect(lines.slice(0, 6)).toEqual(
|
||||
['alpha', 'beta', 'gamma', 'alpha', 'beta', 'gamma']);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue